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

[Graydon Hoare via Phabricator via cfe-commits]

graydon created this revision.

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


https://reviews.llvm.org/D34741

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

Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -126,6 +126,9 @@
 void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData &Data);
 void MergeDefinitionData(ObjCInterfaceDecl *D,
  struct ObjCInterfaceDecl::DefinitionData &&NewDD);
+void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData &Data);
+void MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &&NewDD);
 
 static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader,
  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 &Data) {
 
 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 &&NewDD) {
+  // 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: include/clang/AST/Redeclarable.h
===
--- include/clang/AST/Redeclarable.h
+++ 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.
+//
+//  - Mo

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 104358.
xazax.hun added a comment.

- Removed the whitespace changes
- Factored out one more condition


https://reviews.llvm.org/D34506

Files:
  include/clang/Basic/SourceManager.h
  lib/Basic/SourceManager.cpp

Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -2051,19 +2051,62 @@
   if (LOffs.first.isInvalid() || ROffs.first.isInvalid())
 return LOffs.first.isInvalid() && !ROffs.first.isInvalid();
 
+  std::pair InSameTU = isInTheSameTranslationUnit(LOffs, ROffs);
+  if (InSameTU.first)
+return InSameTU.second;
+
+  // If we arrived here, the location is either in a built-ins buffer or
+  // associated with global inline asm. PR5662 and PR22576 are examples.
+
+  StringRef LB = getBuffer(LOffs.first)->getBufferIdentifier();
+  StringRef RB = getBuffer(ROffs.first)->getBufferIdentifier();
+  bool LIsBuiltins = LB == "";
+  bool RIsBuiltins = RB == "";
+  // Sort built-in before non-built-in.
+  if (LIsBuiltins || RIsBuiltins) {
+if (LIsBuiltins != RIsBuiltins)
+  return LIsBuiltins;
+// Both are in built-in buffers, but from different files. We just claim that
+// lower IDs come first.
+return LOffs.first < ROffs.first;
+  }
+  bool LIsAsm = LB == "";
+  bool RIsAsm = RB == "";
+  // Sort assembler after built-ins, but before the rest.
+  if (LIsAsm || RIsAsm) {
+if (LIsAsm != RIsAsm)
+  return RIsAsm;
+assert(LOffs.first == ROffs.first);
+return false;
+  }
+  bool LIsScratch = LB == "";
+  bool RIsScratch = RB == "";
+  // Sort scratch after inline asm, but before the rest.
+  if (LIsScratch || RIsScratch) {
+if (LIsScratch != RIsScratch)
+  return LIsScratch;
+return LOffs.second < ROffs.second;
+  }
+  llvm_unreachable("Unsortable locations found");
+}
+
+std::pair SourceManager::isInTheSameTranslationUnit(
+std::pair &LOffs,
+std::pair &ROffs) const {
   // If the source locations are in the same file, just compare offsets.
   if (LOffs.first == ROffs.first)
-return LOffs.second < ROffs.second;
+return std::make_pair(true, LOffs.second < ROffs.second);
 
   // If we are comparing a source location with multiple locations in the same
   // file, we get a big win by caching the result.
   InBeforeInTUCacheEntry &IsBeforeInTUCache =
-getInBeforeInTUCache(LOffs.first, ROffs.first);
+  getInBeforeInTUCache(LOffs.first, ROffs.first);
 
   // If we are comparing a source location with multiple locations in the same
   // file, we get a big win by caching the result.
   if (IsBeforeInTUCache.isCacheValid(LOffs.first, ROffs.first))
-return IsBeforeInTUCache.getCachedResult(LOffs.second, ROffs.second);
+return std::make_pair(
+true, IsBeforeInTUCache.getCachedResult(LOffs.second, ROffs.second));
 
   // Okay, we missed in the cache, start updating the cache for this query.
   IsBeforeInTUCache.setQueryFIDs(LOffs.first, ROffs.first,
@@ -2093,44 +2136,12 @@
   // locations within the common file and cache them.
   if (LOffs.first == ROffs.first) {
 IsBeforeInTUCache.setCommonLoc(LOffs.first, LOffs.second, ROffs.second);
-return IsBeforeInTUCache.getCachedResult(LOffs.second, ROffs.second);
+return std::make_pair(
+true, IsBeforeInTUCache.getCachedResult(LOffs.second, ROffs.second));
   }
-
-  // If we arrived here, the location is either in a built-ins buffer or
-  // associated with global inline asm. PR5662 and PR22576 are examples.
-
   // Clear the lookup cache, it depends on a common location.
   IsBeforeInTUCache.clear();
-  StringRef LB = getBuffer(LOffs.first)->getBufferIdentifier();
-  StringRef RB = getBuffer(ROffs.first)->getBufferIdentifier();
-  bool LIsBuiltins = LB == "";
-  bool RIsBuiltins = RB == "";
-  // Sort built-in before non-built-in.
-  if (LIsBuiltins || RIsBuiltins) {
-if (LIsBuiltins != RIsBuiltins)
-  return LIsBuiltins;
-// Both are in built-in buffers, but from different files. We just claim that
-// lower IDs come first.
-return LOffs.first < ROffs.first;
-  }
-  bool LIsAsm = LB == "";
-  bool RIsAsm = RB == "";
-  // Sort assembler after built-ins, but before the rest.
-  if (LIsAsm || RIsAsm) {
-if (LIsAsm != RIsAsm)
-  return RIsAsm;
-assert(LOffs.first == ROffs.first);
-return false;
-  }
-  bool LIsScratch = LB == "";
-  bool RIsScratch = RB == "";
-  // Sort scratch after inline asm, but before the rest.
-  if (LIsScratch || RIsScratch) {
-if (LIsScratch != RIsScratch)
-  return LIsScratch;
-return LOffs.second < ROffs.second;
-  }
-  llvm_unreachable("Unsortable locations found");
+  return std::make_pair(false, false);
 }
 
 void SourceManager::PrintStats() const {
Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D34506#792313, @akyrtzi wrote:

> I'd prefer to avoid including whitespace-only changes (there are a couple of 
> lines in the diff with only whitespace change), otherwise LGTM!


Great, thank you! If no one has objections I will commit this tomorrow.


https://reviews.llvm.org/D34506



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

[PATCH] D31709: [NFC] Refactor DiagnosticRenderer to use FullSourceLoc

[Christof Douma via Phabricator via cfe-commits]

christof marked an inline comment as done.
christof added a comment.

Sorry for missing the API comments. Thanks for the fix chapuni :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D31709



___
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 added inline comments.



Comment at: clangd/ClangdUnit.cpp:276
+class DeclarationLocationsFinder : public index::IndexDataConsumer {
+  std::unique_ptr> DeclarationLocations;
+  const SourceLocation &SearchedLocation;

malaperle-ericsson wrote:
> malaperle-ericsson wrote:
> > ilya-biryukov wrote:
> > > Why do we need to use `unique_ptr>` here and in other 
> > > places instead of `vector`? 
> > It was to implement "takeLocations". Calling it claims ownship of the 
> > vector. Did you have something else in mind when you suggested to implement 
> > takeLocations with a std::move? Disclaimer: this c++11 stuff is all new to 
> > me so I wouldn't be surprised if I'm doing it in a terrible way.
> I guess I can make takeLocations return a vector&& and the other places will 
> return a normal vector RVO will take care of not making copies?
Just return `std::vector` and `std::move` the original field. It will have the 
same behaviour (i.e. no copies of the vector data, ownership transferred to the 
caller) without extra heap allocs:


```
  std::vector takeLocations() {
//  Do postprocessing here, i.e. sort+unique+erase
return std::move(DeclarationLocations);
  }
// ...
private:
  std::vector DeclarationLocations;
```


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] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 104360.
xazax.hun marked 2 inline comments as done.
xazax.hun retitled this revision from "[clang-tidy] Enable constexpr 
definitions in headers. " to "[clang-tidy] Enable inline variable definitions 
in headers. ".
xazax.hun edited the summary of this revision.
xazax.hun added a comment.

- Updates according to the review comments.


https://reviews.llvm.org/D34449

Files:
  clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/misc-definitions-in-headers-1z.hpp
  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
@@ -175,3 +175,5 @@
 int CD::f() { // OK: partial template specialization.
   return 0;
 }
+
+constexpr int k = 1; // OK: constexpr variable has internal linkage.
Index: test/clang-tidy/misc-definitions-in-headers-1z.hpp
===
--- /dev/null
+++ 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: 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
@@ -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-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ 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: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -175,3 +175,5 @@
 int CD::f() { // OK: partial template specialization.
   return 0;
 }
+
+constexpr int k = 1; // OK: constexpr variable has internal linkage.
Index: test/clang-tidy/misc-definitions-in-headers-1z.hpp
===
--- /dev/null
+++ 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: 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
@@ -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-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ 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; "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r306519 - Fix crash in clang while handling __has_trivial_destructor.

[Karthik Bhat via cfe-commits]

Author: karthik
Date: Wed Jun 28 01:52:08 2017
New Revision: 306519

URL: http://llvm.org/viewvc/llvm-project?rev=306519&view=rev
Log:
Fix crash in clang while handling __has_trivial_destructor.

Fix crash in clang when an array of unknown bounds of an incomplete type is 
passed to __has_trivial_destructor.

Patch by Puneetha
https://reviews.llvm.org/D34198


Modified:
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/type-traits.cpp

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=306519&r1=306518&r2=306519&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jun 28 01:52:08 2017
@@ -4093,15 +4093,9 @@ static bool CheckUnaryTypeTraitTypeCompl
   case UTT_IsStandardLayout:
   case UTT_IsPOD:
   case UTT_IsLiteral:
-ArgTy = QualType(ArgTy->getBaseElementTypeUnsafe(), 0);
-LLVM_FALLTHROUGH;
-
-  // C++1z [meta.unary.prop]:
-  //   T shall be a complete type, cv void, or an array of unknown bound.
-  case UTT_IsDestructible:
-  case UTT_IsNothrowDestructible:
-  case UTT_IsTriviallyDestructible:
-  // Per the GCC type traits documentation, the same constraints apply to 
these.
+  // Per the GCC type traits documentation, T shall be a complete type, cv 
void,
+  // or an array of unknown bound. But GCC actually imposes the same 
constraints
+  // as above.
   case UTT_HasNothrowAssign:
   case UTT_HasNothrowMoveAssign:
   case UTT_HasNothrowConstructor:
@@ -4113,6 +4107,14 @@ static bool CheckUnaryTypeTraitTypeCompl
   case UTT_HasTrivialCopy:
   case UTT_HasTrivialDestructor:
   case UTT_HasVirtualDestructor:
+ArgTy = QualType(ArgTy->getBaseElementTypeUnsafe(), 0);
+LLVM_FALLTHROUGH;
+
+  // C++1z [meta.unary.prop]:
+  //   T shall be a complete type, cv void, or an array of unknown bound.
+  case UTT_IsDestructible:
+  case UTT_IsNothrowDestructible:
+  case UTT_IsTriviallyDestructible:
 if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType())
   return true;
 

Modified: cfe/trunk/test/SemaCXX/type-traits.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=306519&r1=306518&r2=306519&view=diff
==
--- cfe/trunk/test/SemaCXX/type-traits.cpp (original)
+++ cfe/trunk/test/SemaCXX/type-traits.cpp Wed Jun 28 01:52:08 2017
@@ -1447,7 +1447,9 @@ void has_trivial_default_constructor() {
   { int arr[T(__has_trivial_constructor(const Int))]; }
   { int arr[T(__has_trivial_constructor(AllDefaulted))]; }
   { int arr[T(__has_trivial_constructor(AllDeleted))]; }
+  { int arr[T(__has_trivial_constructor(ACompleteType[]))]; }
 
+  { int arr[F(__has_trivial_constructor(AnIncompleteType[]))]; } // 
expected-error {{incomplete type}}
   { int arr[F(__has_trivial_constructor(HasCons))]; }
   { int arr[F(__has_trivial_constructor(HasRef))]; }
   { int arr[F(__has_trivial_constructor(HasCopy))]; }
@@ -1478,7 +1480,9 @@ void has_trivial_move_constructor() {
   { int arr[T(__has_trivial_move_constructor(HasCons))]; }
   { int arr[T(__has_trivial_move_constructor(HasStaticMemberMoveCtor))]; }
   { int arr[T(__has_trivial_move_constructor(AllDeleted))]; }
-  
+  { int arr[T(__has_trivial_move_constructor(ACompleteType[]))]; }
+
+  { int arr[F(__has_trivial_move_constructor(AnIncompleteType[]))]; } // 
expected-error {{incomplete type}}
   { int arr[F(__has_trivial_move_constructor(HasVirt))]; }
   { int arr[F(__has_trivial_move_constructor(DerivesVirt))]; }
   { int arr[F(__has_trivial_move_constructor(HasMoveCtor))]; }
@@ -1508,7 +1512,9 @@ void has_trivial_copy_constructor() {
   { int arr[T(__has_trivial_copy(AllDeleted))]; }
   { int arr[T(__has_trivial_copy(DerivesAr))]; }
   { int arr[T(__has_trivial_copy(DerivesHasRef))]; }
+  { int arr[T(__has_trivial_copy(ACompleteType[]))]; }
 
+  { int arr[F(__has_trivial_copy(AnIncompleteType[]))]; } // expected-error 
{{incomplete type}}
   { int arr[F(__has_trivial_copy(HasCopy))]; }
   { int arr[F(__has_trivial_copy(HasTemplateCons))]; }
   { int arr[F(__has_trivial_copy(VirtAr))]; }
@@ -1536,7 +1542,9 @@ void has_trivial_copy_assignment() {
   { int arr[T(__has_trivial_assign(AllDeleted))]; }
   { int arr[T(__has_trivial_assign(DerivesAr))]; }
   { int arr[T(__has_trivial_assign(DerivesHasRef))]; }
+  { int arr[T(__has_trivial_assign(ACompleteType[]))]; }
 
+  { int arr[F(__has_trivial_assign(AnIncompleteType[]))]; } // expected-error 
{{incomplete type}}
   { int arr[F(__has_trivial_assign(IntRef))]; }
   { int arr[F(__has_trivial_assign(HasCopyAssign))]; }
   { int arr[F(__has_trivial_assign(const Int))]; }
@@ -1572,8 +1580,10 @@ void has_trivial_destructor() {
   { int arr[T(__has_trivial_destructor(AllDefaulted))]; }
   { int arr[T(__has_trivial_destructor(AllDeleted))]; }
   { int arr[T(__has_trivial_destructor(DerivesHasRe

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

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D33440



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

[PATCH] D34686: [AArch64] Add hasFP16VectorArithmetic helper function. NFCI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I agree with the spirit of the change but I think you want to use `AArch64::` 
enumerators instead of those in `ARM::`




Comment at: lib/Basic/Targets.cpp:6190
   unsigned HasFullFP16;
+  unsigned ArchKind;
 

There is an `AArch64::ArchKind` enum in TargetParser.h, not sure if it would be 
useable here. 



Comment at: lib/Basic/Targets.cpp:6332
 
-if (V8_1A)
+if (ArchKind == llvm::ARM::AK_ARMV8_1A)
   Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");

I wonder if this is wrong, if so please fix it in a further change otherwise 
this one wouldn't be a NFCI.



Comment at: lib/Basic/Targets.cpp:6364
 HasFullFP16 = 0;
+ArchKind = llvm::ARM::AK_INVALID;
 

Would it make sense to set this to `AArch64::AK_ARMV8A` instead?



Comment at: lib/Basic/Targets.cpp:6376
   if (Feature == "+v8.1a")
-V8_1A = 1;
+ArchKind = llvm::ARM::AK_ARMV8_1A;
   if (Feature == "+v8.2a")

I think you want to use `llvm::AArch64::AK_ARMV8_1A` instead here.



Comment at: lib/Basic/Targets.cpp:6378
   if (Feature == "+v8.2a")
-V8_2A = 1;
+ArchKind = llvm::ARM::AK_ARMV8_2A;
   if (Feature == "+fullfp16")

Ditto with `llvm::AArch64::AK_ARMV8_2A`


https://reviews.llvm.org/D34686



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

[PATCH] D34687: [Tooling] CompilationDatabase should be able to strip position arguments when `-fsyntax-only` is used

[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


Repository:
  rL LLVM

https://reviews.llvm.org/D34687



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

[clang-tools-extra] r306530 - [clangd] Allow to override resource dir in ClangdServer.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Jun 28 03:34:50 2017
New Revision: 306530

URL: http://llvm.org/viewvc/llvm-project?rev=306530&view=rev
Log:
[clangd] Allow to override resource dir in ClangdServer.

Reviewers: bkramer, krasimir, klimek

Reviewed By: klimek

Subscribers: klimek, cfe-commits

Tags: #clang-tools-extra

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

Modified:
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/ClangdUnitStore.h

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=306530&r1=306529&r2=306530&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Wed Jun 28 03:34:50 2017
@@ -33,6 +33,11 @@ std::vector format
   return std::vector(Result.begin(), Result.end());
 }
 
+std::string getStandardResourceDir() {
+  static int Dummy; // Just an address in this process.
+  return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
+}
+
 } // namespace
 
 size_t clangd::positionToOffset(StringRef Code, Position P) {
@@ -141,8 +146,10 @@ void ClangdScheduler::addToEnd(std::func
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider,
-   bool RunSynchronously)
+   bool RunSynchronously,
+   llvm::Optional ResourceDir)
 : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
+  ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
   PCHs(std::make_shared()),
   WorkScheduler(RunSynchronously) {}
 
@@ -158,7 +165,7 @@ void ClangdServer::addDocument(PathRef F
"No contents inside a file that was scheduled for reparse");
 auto TaggedFS = FSProvider.getTaggedFileSystem(FileStr);
 Units.runOnUnit(
-FileStr, *FileContents.Draft, CDB, PCHs, TaggedFS.Value,
+FileStr, *FileContents.Draft, ResourceDir, CDB, PCHs, TaggedFS.Value,
 [&](ClangdUnit const &Unit) {
   DiagConsumer.onDiagnosticsReady(
   FileStr, make_tagged(Unit.getLocalDiagnostics(), TaggedFS.Tag));
@@ -201,8 +208,8 @@ ClangdServer::codeComplete(PathRef File,
   // It would be nice to use runOnUnitWithoutReparse here, but we can't
   // guarantee the correctness of code completion cache here if we don't do the
   // reparse.
-  Units.runOnUnit(File, *OverridenContents, CDB, PCHs, TaggedFS.Value,
-  [&](ClangdUnit &Unit) {
+  Units.runOnUnit(File, *OverridenContents, ResourceDir, CDB, PCHs,
+  TaggedFS.Value, [&](ClangdUnit &Unit) {
 Result = Unit.codeComplete(*OverridenContents, Pos,
TaggedFS.Value);
   });

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=306530&r1=306529&r2=306530&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Jun 28 03:34:50 2017
@@ -150,9 +150,14 @@ public:
   /// a vfs::FileSystem provided by \p FSProvider. Results of code
   /// completion/diagnostics also include a tag, that \p FSProvider returns
   /// along with the vfs::FileSystem.
+  /// When \p ResourceDir is set, it will be used to search for internal 
headers
+  /// (overriding defaults and -resource-dir compiler flag, if set). If \p
+  /// ResourceDir is None, ClangdServer will attempt to set it to a standard
+  /// location, obtained via CompilerInvocation::GetResourcePath.
   ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
-   FileSystemProvider &FSProvider, bool RunSynchronously);
+   FileSystemProvider &FSProvider, bool RunSynchronously,
+   llvm::Optional ResourceDir = llvm::None);
 
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
@@ -199,6 +204,7 @@ private:
   FileSystemProvider &FSProvider;
   DraftStore DraftMgr;
   ClangdUnitStore Units;
+  std::string ResourceDir;
   std::shared_ptr PCHs;
   // WorkScheduler has to be the last member, because its destructor has to be
   // called before all other members to stop the worker thread that references

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-t

[PATCH] D34470: [clangd] Allow to override resource dir in ClangdServer.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL306530: [clangd] Allow to override resource dir in 
ClangdServer. (authored by ibiryukov).

Repository:
  rL LLVM

https://reviews.llvm.org/D34470

Files:
  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/ClangdUnitStore.h

Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -44,7 +44,7 @@
 /// would want to perform on parsed C++ files.
 class ClangdUnit {
 public:
-  ClangdUnit(PathRef FileName, StringRef Contents,
+  ClangdUnit(PathRef FileName, StringRef Contents, StringRef ResourceDir,
  std::shared_ptr PCHs,
  std::vector Commands,
  IntrusiveRefCntPtr VFS);
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -19,18 +19,16 @@
 using namespace clang;
 
 ClangdUnit::ClangdUnit(PathRef FileName, StringRef Contents,
+   StringRef ResourceDir,
std::shared_ptr PCHs,
std::vector Commands,
IntrusiveRefCntPtr VFS)
 : FileName(FileName), PCHs(PCHs) {
   assert(!Commands.empty() && "No compile commands provided");
 
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
-  static int Dummy; // Just an address in this process.
-  std::string ResourceDir =
-  CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
-  Commands.front().CommandLine.push_back("-resource-dir=" + ResourceDir);
+  Commands.front().CommandLine.push_back("-resource-dir=" + std::string(ResourceDir));
 
   IntrusiveRefCntPtr Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions);
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -150,9 +150,14 @@
   /// a vfs::FileSystem provided by \p FSProvider. Results of code
   /// completion/diagnostics also include a tag, that \p FSProvider returns
   /// along with the vfs::FileSystem.
+  /// When \p ResourceDir is set, it will be used to search for internal headers
+  /// (overriding defaults and -resource-dir compiler flag, if set). If \p
+  /// ResourceDir is None, ClangdServer will attempt to set it to a standard
+  /// location, obtained via CompilerInvocation::GetResourcePath.
   ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
-   FileSystemProvider &FSProvider, bool RunSynchronously);
+   FileSystemProvider &FSProvider, bool RunSynchronously,
+   llvm::Optional ResourceDir = llvm::None);
 
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of the AST for it on a
@@ -199,6 +204,7 @@
   FileSystemProvider &FSProvider;
   DraftStore DraftMgr;
   ClangdUnitStore Units;
+  std::string ResourceDir;
   std::shared_ptr PCHs;
   // WorkScheduler has to be the last member, because its destructor has to be
   // called before all other members to stop the worker thread that references
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -33,6 +33,11 @@
   return std::vector(Result.begin(), Result.end());
 }
 
+std::string getStandardResourceDir() {
+  static int Dummy; // Just an address in this process.
+  return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
+}
+
 } // namespace
 
 size_t clangd::positionToOffset(StringRef Code, Position P) {
@@ -141,8 +146,10 @@
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider,
-   bool RunSynchronously)
+   bool RunSynchronously,
+   llvm::Optional ResourceDir)
 : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
+  ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
   PCHs(std::make_shared()),
   WorkScheduler(RunSynchronously) {}
 
@@ -158,7 +165,7 @@
"No contents inside a file that was scheduled for reparse");
 auto TaggedFS = FSProvider.getTaggedFileSystem(FileStr);
 Units.runOnU

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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 104379.
xazax.hun added a comment.

- Unbreak the constexpr test.


https://reviews.llvm.org/D34449

Files:
  clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  docs/clang-tidy/checks/misc-definitions-in-headers.rst
  test/clang-tidy/misc-definitions-in-headers-1z.hpp
  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
+// 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: test/clang-tidy/misc-definitions-in-headers-1z.hpp
===
--- /dev/null
+++ 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: 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
@@ -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-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ 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: 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
+// 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: test/clang-tidy/misc-definitions-in-headers-1z.hpp
===
--- /dev/null
+++ 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: 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
@@ -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-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -139,6 +139,9 @@
 // Ignore variable definition within function sco

[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#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?


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] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:1
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++1z
 

hokein wrote:
> hokein wrote:
> > The original code should work as `-std=c++11` will be added defaultly by 
> > `check_clang_tidy` script.
> `constexpr` variables have internal linkage, which should be detected for the 
> current check (but the test case is missing this kind of case).
> 
> If you want to test `inline` variables, I'd suggest adding a new test file 
> like `misc-definitions-in-headers-1z.hpp` which includes cases of inline 
> variables.
It looks like `-std=c++11` is not added. 



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

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. 


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] D34198: Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.

[Puneetha K via Phabricator via cfe-commits]

puneetha added a comment.

This is committed by karthik, at r306519.

Thank you.


https://reviews.llvm.org/D34198



___
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#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.


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] D34686: [AArch64] Add hasFP16VectorArithmetic helper function. NFCI

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 104380.
SjoerdMeijer added a comment.

Thanks! I am now using llvm::AArch64::ArchKind. And I agree that the check for 
setting __ARM_FEATURE_QRDMX is suspicious. I will address this separately.


https://reviews.llvm.org/D34686

Files:
  lib/Basic/Targets.cpp


Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -6186,9 +6186,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 +6253,18 @@
static_cast(llvm::AArch64::ArchKind::AK_INVALID);
   }
 
+  bool hasFP16VectorArithmetic() const {
+if (FPU != NeonMode || !HasFullFP16)
+  return false;
+
+switch(ArchKind) {
+default:
+  return false;
+case llvm::AArch64::ArchKind::AK_ARMV8_2A:
+  return true;
+}
+  }
+
   void getTargetDefines(const LangOptions &Opts,
 MacroBuilder &Builder) const override {
 // Target identification.
@@ -6318,9 +6329,10 @@
 if (Unaligned)
   Builder.defineMacro("__ARM_FEATURE_UNALIGNED", "1");
 
-if (V8_1A)
+if (ArchKind == llvm::AArch64::ArchKind::AK_ARMV8_1A)
   Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
-if (V8_2A && FPU == NeonMode && HasFullFP16)
+
+if (hasFP16VectorArithmetic())
   Builder.defineMacro("__ARM_FEATURE_FP16_VECTOR_ARITHMETIC", "1");
 
 // All of the __sync_(bool|val)_compare_and_swap_(1|2|4|8) builtins work.
@@ -6348,9 +6360,8 @@
 CRC = 0;
 Crypto = 0;
 Unaligned = 1;
-V8_1A = 0;
-V8_2A = 0;
 HasFullFP16 = 0;
+ArchKind = llvm::AArch64::ArchKind::AK_ARMV8A;
 
 for (const auto &Feature : Features) {
   if (Feature == "+neon")
@@ -6362,9 +6373,9 @@
   if (Feature == "+strict-align")
 Unaligned = 0;
   if (Feature == "+v8.1a")
-V8_1A = 1;
+ArchKind = llvm::AArch64::ArchKind::AK_ARMV8_1A;
   if (Feature == "+v8.2a")
-V8_2A = 1;
+ArchKind = llvm::AArch64::ArchKind::AK_ARMV8_2A;
   if (Feature == "+fullfp16")
 HasFullFP16 = 1;
 }


Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -6186,9 +6186,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 +6253,18 @@
static_cast(llvm::AArch64::ArchKind::AK_INVALID);
   }
 
+  bool hasFP16VectorArithmetic() const {
+if (FPU != NeonMode || !HasFullFP16)
+  return false;
+
+switch(ArchKind) {
+default:
+  return false;
+case llvm::AArch64::ArchKind::AK_ARMV8_2A:
+  return true;
+}
+  }
+
   void getTargetDefines(const LangOptions &Opts,
 MacroBuilder &Builder) const override {
 // Target identification.
@@ -6318,9 +6329,10 @@
 if (Unaligned)
   Builder.defineMacro("__ARM_FEATURE_UNALIGNED", "1");
 
-if (V8_1A)
+if (ArchKind == llvm::AArch64::ArchKind::AK_ARMV8_1A)
   Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
-if (V8_2A && FPU == NeonMode && HasFullFP16)
+
+if (hasFP16VectorArithmetic())
   Builder.defineMacro("__ARM_FEATURE_FP16_VECTOR_ARITHMETIC", "1");
 
 // All of the __sync_(bool|val)_compare_and_swap_(1|2|4|8) builtins work.
@@ -6348,9 +6360,8 @@
 CRC = 0;
 Crypto = 0;
 Unaligned = 1;
-V8_1A = 0;
-V8_2A = 0;
 HasFullFP16 = 0;
+ArchKind = llvm::AArch64::ArchKind::AK_ARMV8A;
 
 for (const auto &Feature : Features) {
   if (Feature == "+neon")
@@ -6362,9 +6373,9 @@
   if (Feature == "+strict-align")
 Unaligned = 0;
   if (Feature == "+v8.1a")
-V8_1A = 1;
+ArchKind = llvm::AArch64::ArchKind::AK_ARMV8_1A;
   if (Feature == "+v8.2a")
-V8_2A = 1;
+ArchKind = llvm::AArch64::ArchKind::AK_ARMV8_2A;
   if (Feature == "+fullfp16")
 HasFullFP16 = 1;
 }
___
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#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.


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] D34449: [clang-tidy] Enable inline variable definitions in headers.

[Haojian Wu via Phabricator via cfe-commits]

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

Looks good, thanks!




Comment at: test/clang-tidy/misc-definitions-in-headers.hpp:1
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -- -std=c++1z
 

xazax.hun wrote:
> hokein wrote:
> > hokein wrote:
> > > The original code should work as `-std=c++11` will be added defaultly by 
> > > `check_clang_tidy` script.
> > `constexpr` variables have internal linkage, which should be detected for 
> > the current check (but the test case is missing this kind of case).
> > 
> > If you want to test `inline` variables, I'd suggest adding a new test file 
> > like `misc-definitions-in-headers-1z.hpp` which includes cases of inline 
> > variables.
> It looks like `-std=c++11` is not added. 
OK. Yes, you are right.

The `-std=c++11` only be added if the test file extension is `cpp`. I think we 
can extend the check_clang_tidy.py script to support it, but it can be done in 
a follow-up.


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] 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

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

[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&view=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&r1=306537&r2=306538&view=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&r1=306537&r2=306538&view=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&view=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&r1=306537&r2=306538&view=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] 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&id=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 misc-definitions-in-headers %

[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] 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 *&NewDisp);
   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 &Info, 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 &Info = 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 &SMInfo = 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 t

[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 &Match : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto &Change : 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(cla

[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 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 &Match : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto &Change : 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: test/Tooling/clang-diff-basic.cpp
=

[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 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 &Match : DiffTool.getMatches())
+DiffTool.printMatch(llvm::outs(), Match);
+  for (const auto &Change : 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: test/Tooling/clang-diff-basic.cpp
=

[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 &Opts,
+   MacroBuilder &Builder) const {
+Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
+  }
+
+  void getTargetDefinesARMV82A(const LangOptions &Opts,
+   MacroBuilder &Builder) const {
+// Also include the ARMv8.1-A defines
+getTargetDefinesARMV81A(Opts, Builder);
+  }
+
   void getTargetDefines(const LangOptions &Opts,
 MacroBuilder &Builder) 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 &Opts,
+MacroBuilder &Builder) const {
+Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
+  }
+
+  void getTargetDefinesARMV82A(const LangOptions &Opts,
+MacroBuilder &Builder) 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 &Opts,
 MacroBuilder &Builder) 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(Ar

[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] 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] 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] 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] 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
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","param

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

[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] 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] 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] 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 &Items, 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] 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 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] 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.

[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

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&view=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&r1=306548&r2=306549&view=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&r1=306548&r2=306549&view=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(
   &StoredDiagnostics, 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&r1=306548&r2=306549&view=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] 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 @@
   &StoredDiagnostics, 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 @@
   &StoredDiagnostics, nullptr);
 
 CI = clang::createInvocationFromCommandLine(
-   

[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 &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args) const override;
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) 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 &DriverArgs,
-   ArgStringList &CC1Args) const {
+   ArgStringList &CC1Args,
+   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 &DriverArgs,
- llvm::opt::ArgStringList &CC1Args) const override;
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const override;
   RuntimeLibType GetDefaultRuntimeLibType() const override;
   CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) 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 &DriverArgs,
-ArgStringList &CC1Args) const {
+ArgStringList &CC1Args,
+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 &DriverArgs,
- llvm::opt::ArgStringList &CC1Args) const override;
+   

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

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&view=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&r1=306554&r2=306555&view=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] 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&id=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

[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

[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&view=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&r1=306557&r2=306558&view=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&r1=306557&r2=306558&view=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 &Out) override;
   void onCompletion(TextDocumentPositionParams Params, StringRef ID,
 JSONOutput &Out) override;
+  void onGoToDefinition(TextDocumentPositionParams Params, StringRef ID,
+JSONOutput &Out) override;
 
 private:
   ClangdLSPServer &LangServer;
@@ -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 &Out) {
+
+  auto Items = LangServer.Server.findDefinitions(
+  Params.textDocument.uri.file,
+  Position{Params.position.line, Params.position.character}).Value;
+
+  std::string Locations;
+  for (const auto &Item : 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 &Out, 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&r1=306557&r2=306558&view=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.

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

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

[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

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&view=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&r1=306558&r2=306559&view=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

[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&id=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

[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] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In https://reviews.llvm.org/D34158#792858, @fedor.sergeev wrote: >

> I will take a look at the final version tomorrow.


Fedor, let me address the comments from Jonas (with another revision!) before 
you take a look.


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&view=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&r1=306126&r2=306127&view=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 fconstant_str

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 &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) 
+ 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 &DriverArgs, 
+ArgStringList &CC1Args) const {
+  const Generic_GCC::GCCVersion &Version = 
+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 &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) 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

[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

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

[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 &Category : 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] 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] 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

[libcxx] r306580 - Added failing tests for index out of range for tuple_element> 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&view=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&view=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&view=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

[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&view=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&r1=306580&r2=306581&view=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

[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

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

[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

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&view=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&r1=306582&r2=306583&view=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 {

Modifi

[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 &Data);
 void MergeDefinitionData(ObjCInterfaceDecl *D,
  struct ObjCInterfaceDecl::DefinitionData &&NewDD);
+void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData &Data);
+void MergeDefinitionData(ObjCProtocolDecl *D,
+ struct ObjCProtocolDecl::DefinitionData &&NewDD);
 
 static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader,
  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 &Data) {
 
 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 &&NewDD) {
+  // 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 

[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] 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] 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] 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 &In : 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
http:/

[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 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 &In : 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] 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

[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&view=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&r1=306597&r2=306598&view=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 &OS,
   const SourceLocation &SearchedLocation, ASTUnit &Unit) :
-  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&r1=306597&r2=306598&view=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] 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 -che

[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 

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&view=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&r1=306582&r2=306583&view=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] 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

[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 &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
- llvm::opt::ArgStringList &CC1Args) const override;
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void AddClangCXXStdlibIncludeArgs(
   const llvm::opt::ArgList &DriverArgs,
   llvm::opt::ArgStringList &CC1Args) 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 &DriverArgs,
-   ArgStringList &CC1Args) const {
+   ArgStringList &CC1Args,
+   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 &DriverArgs,
- llvm::opt::ArgStringList &CC1Args) const override;
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const override;
   RuntimeLibType GetDefaultRuntimeLibType() const override;
   CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) 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 &DriverArgs,
-ArgStringList &CC1Args) const {
+ArgStringList &CC1Args,
+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 &DriverArgs,
- llvm::opt::ArgStringList &CC1Args) const override;
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const override;
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) 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 &DriverArgs,
- ArgStringList &CC1Args) const {
+ ArgStringList &CC1Args,
+ Action::OffloadKind) const {
   if (D

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

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

[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 &Output, OptSpecifier Id) const {
-  if (Arg *A = getLastArg(Id)) {
-A->claim();
-A->render(*this, Output);
-  }
-}
-
-void ArgList::AddLastArg(ArgStringList &Output, OptSpecifier Id0,
- OptSpecifier Id1) const {
-  if (Arg *A = getLastArg(Id0, Id1)) {
-A->claim();
-A->render(*this, Output);
-  }
-}
-
 void ArgList::AddAllArgsExcept(ArgStringList &Output,
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 &Output, OptSpecifier Id0) const;
-  void AddLastArg(ArgStringList &Output, OptSpecifier Id0,
-  OptSpecifier Id1) const;
+  template 
+  void AddLastArg(Arg

[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

[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&view=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&r1=306611&r2=306612&view=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] 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 &TC, Compilation &C,
-  const InputInfo &Output,
-  const InputInfoList &Inputs,
-  const ArgList &Args, ArgStringList &CmdArgs,
-  const JobAction &JA) {
-
-  // 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 ca