[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev added a comment.

Correct, they are eliminated. However, this change makes things a bit easier as 
we are pursuing undecorated instructions produced by clang. Once they have 
their TBAA tags, we don't need to guess if it's something trivial for the 
optimizer. It shouldn't be a problem to keep this as a local patch, though. So 
I'm fine with either way.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



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

[libunwind] r316415 - Add missing checks for register number

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Tue Oct 24 00:16:40 2017
New Revision: 316415

URL: http://llvm.org/viewvc/llvm-project?rev=316415&view=rev
Log:
Add missing checks for register number

Most other cases that touch savedRegisters[reg] have got this check,
but these three seemed to lack it.

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

Modified:
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=316415&r1=316414&r2=316415&view=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Tue Oct 24 00:16:40 2017
@@ -605,6 +605,13 @@ bool CFI_Parser::parseInstructions(A
   break;
 case DW_CFA_val_offset:
   reg = addressSpace.getULEB128(p, instructionsEnd);
+  if (reg > kMaxRegisterNumber) {
+fprintf(stderr,
+"malformed DW_CFA_val_offset DWARF unwind, reg (%" PRIu64
+") out of range\n",
+reg);
+return false;
+  }
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
   results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
@@ -668,6 +675,12 @@ bool CFI_Parser::parseInstructions(A
   switch (opcode & 0xC0) {
   case DW_CFA_offset:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr, "malformed DW_CFA_offset DWARF unwind, reg (%" PRIu64
+  ") out of range\n",
+  reg);
+  return false;
+}
 offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
 results->savedRegisters[reg].location = kRegisterInCFA;
@@ -682,6 +695,12 @@ bool CFI_Parser::parseInstructions(A
 break;
   case DW_CFA_restore:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr, "malformed DW_CFA_restore DWARF unwind, reg (%" 
PRIu64
+  ") out of range\n",
+  reg);
+  return false;
+}
 results->savedRegisters[reg] = initialState.savedRegisters[reg];
 _LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
static_cast(operand));


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

[PATCH] D39206: [libunwind] Add missing checks for register number

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316415: Add missing checks for register number (authored by 
mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D39206?vs=119940&id=120001#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39206

Files:
  libunwind/trunk/src/DwarfParser.hpp


Index: libunwind/trunk/src/DwarfParser.hpp
===
--- libunwind/trunk/src/DwarfParser.hpp
+++ libunwind/trunk/src/DwarfParser.hpp
@@ -605,6 +605,13 @@
   break;
 case DW_CFA_val_offset:
   reg = addressSpace.getULEB128(p, instructionsEnd);
+  if (reg > kMaxRegisterNumber) {
+fprintf(stderr,
+"malformed DW_CFA_val_offset DWARF unwind, reg (%" PRIu64
+") out of range\n",
+reg);
+return false;
+  }
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
   results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
@@ -668,6 +675,12 @@
   switch (opcode & 0xC0) {
   case DW_CFA_offset:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr, "malformed DW_CFA_offset DWARF unwind, reg (%" PRIu64
+  ") out of range\n",
+  reg);
+  return false;
+}
 offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
 results->savedRegisters[reg].location = kRegisterInCFA;
@@ -682,6 +695,12 @@
 break;
   case DW_CFA_restore:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr, "malformed DW_CFA_restore DWARF unwind, reg (%" 
PRIu64
+  ") out of range\n",
+  reg);
+  return false;
+}
 results->savedRegisters[reg] = initialState.savedRegisters[reg];
 _LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
static_cast(operand));


Index: libunwind/trunk/src/DwarfParser.hpp
===
--- libunwind/trunk/src/DwarfParser.hpp
+++ libunwind/trunk/src/DwarfParser.hpp
@@ -605,6 +605,13 @@
   break;
 case DW_CFA_val_offset:
   reg = addressSpace.getULEB128(p, instructionsEnd);
+  if (reg > kMaxRegisterNumber) {
+fprintf(stderr,
+"malformed DW_CFA_val_offset DWARF unwind, reg (%" PRIu64
+") out of range\n",
+reg);
+return false;
+  }
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
   results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
@@ -668,6 +675,12 @@
   switch (opcode & 0xC0) {
   case DW_CFA_offset:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr, "malformed DW_CFA_offset DWARF unwind, reg (%" PRIu64
+  ") out of range\n",
+  reg);
+  return false;
+}
 offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
 results->savedRegisters[reg].location = kRegisterInCFA;
@@ -682,6 +695,12 @@
 break;
   case DW_CFA_restore:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr, "malformed DW_CFA_restore DWARF unwind, reg (%" PRIu64
+  ") out of range\n",
+  reg);
+  return false;
+}
 results->savedRegisters[reg] = initialState.savedRegisters[reg];
 _LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
static_cast(operand));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39206: [libunwind] Add missing checks for register number

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D39206#904723, @compnerd wrote:

> I'd say out of range rather than too big.


Ok, pushing with the message changed to out of range, and actually printing the 
value.


Repository:
  rL LLVM

https://reviews.llvm.org/D39206



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

[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay, if this is just for your own checking, I'd rather not take it.  It's not 
a significant compile-time cost, but there's no reason to pay it at all.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



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

[PATCH] D39086: Performance tracing facility for clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Trace.cpp:87
+
+static Tracer* T = nullptr;
+} // namespace

sammccall wrote:
> ilya-biryukov wrote:
> > Maybe use `unique_ptr` (or even `llvm::Optional`)? 
> > Otherwise we leak memory and don't flush the stream if users of tracing API 
> > forget to call `stop()`. (I think I'm on the same page with @ioeric here  
> > about forgetting a call to `stop()`).
> >  
> > Having a static global of non-pod type is a no-go, but we could use static 
> > local variables:
> > 
> > ```
> > llvm::Optional& getTracerInstance() {
> >  static llvm::Optional Tracer;
> >  return Tracer;
> > }
> > ```
> So Eric's solution of a scoped object that would be owned by main() makes 
> sense to me, and I'm happy to add it (though personally not sure it carries 
> its weight).
> 
> However I don't understand *your* suggestion :-)
> > Having a static global of non-pod type is a no-go, but we could use static 
> > local variables:
> > llvm::Optional& getTracerInstance() {
> >   static llvm::Optional Tracer;
> >   return Tracer;
> > }
> So it's not global, which means we're not calling Optional() on startup, 
> which would actually be totally safe (I guess the constructor should be 
> marked as `constexpr`).
> 
> But we *are* calling `~Optional()` and therefore `~Tracer()` at shutdown. And 
> maybe `~Tracer()` calls something that logs, and llvm::errs() has already 
> been destroyed... *this* is why avoid static objects, right?
> 
> > Otherwise we leak memory and don't flush the stream if users of tracing API 
> > forget to call stop()
> The intended time to call `stop()` is at the end of the program. If you 
> forget to call it, we won't be leaking memory :-) We indeed won't flush the 
> stream, but that wouldn't be safe (see above).
> 
> The other option is the user is calling `start()` and `stop()` multiple 
> times, and they call `start()` twice without stopping. We already assert on 
> this, and anyone who doesn't reproducibly hit that assert is well into racy 
> UB-land (logging methods may be called concurrently with each other, but 
> everything else must be synchronized).
> 
> Overall I'm not really seeing what scenario we can detect and safely do 
> something about.
Forget what I said, you're right. Tracer has state, can be reconfigured 
multiple times and there's no guarantee our references won't be dangling on 
destruction of `Tracer`.

I do think Eric's solution is good, though. For all the same reasons why RAII 
is good.


https://reviews.llvm.org/D39086



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

[PATCH] D36111: [COFF, ARM64] Add MS builtins __dmb, __dsb, __isb

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo abandoned this revision.
mstorsjo added a comment.

This was updated, finished and merged in https://reviews.llvm.org/D38821.


https://reviews.llvm.org/D36111



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

[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

> One of the possible improvements for future work here would be to actually 
> bind the second argument value to the buffer instead of just invalidating it. 
> Like, after `memset(buf, 0, sizeof(buf))` the analyzer should know that all 
> values in the `buf` array are `0`. In the analyzer we have the notion of 
> *default bindings* to handle that (see documentation in 
> docs/analyzer/RegionStore.txt for more details).

`BindDefault()` is the only function that can make the default binding, is it? 
If so, `evalMemset()` uses `bindDefault()`, the binding may not take effect. 
Because the current `BindDefault()` logic is that if the memory area has been 
initialized, then the default binding will no longer be done, see 
https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Core/RegionStore.cpp#L429.
 Before `evalMemset()`, `MallocMemAux()` in MallocChecker.cpp may have already 
made the default binding. Am I right?

Thank you!


Repository:
  rL LLVM

https://reviews.llvm.org/D31868



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

[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
Herald added a subscriber: mgorny.

For code reuse in SemaCodeComplete.
Note that the tests for QualTypeNames are still in Tooling as they use
Tooling's common testing code.


https://reviews.llvm.org/D39224

Files:
  include/clang/AST/QualTypeNames.h
  include/clang/Tooling/Core/QualTypeNames.h
  lib/AST/CMakeLists.txt
  lib/AST/QualTypeNames.cpp
  lib/Tooling/Core/CMakeLists.txt
  lib/Tooling/Core/QualTypeNames.cpp
  unittests/Tooling/QualTypeNamesTest.cpp

Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -7,7 +7,7 @@
 //
 //===--===//
 
-#include "clang/Tooling/Core/QualTypeNames.h"
+#include "clang/AST/QualTypeNames.h"
 #include "TestVisitor.h"
 using namespace clang;
 
Index: lib/Tooling/Core/CMakeLists.txt
===
--- lib/Tooling/Core/CMakeLists.txt
+++ lib/Tooling/Core/CMakeLists.txt
@@ -3,7 +3,6 @@
 add_clang_library(clangToolingCore
   Lookup.cpp
   Replacement.cpp
-  QualTypeNames.cpp
   Diagnostic.cpp
 
   LINK_LIBS
Index: lib/AST/QualTypeNames.cpp
===
--- lib/AST/QualTypeNames.cpp
+++ lib/AST/QualTypeNames.cpp
@@ -9,29 +9,18 @@
 //
 //===--===//
 
-#include "clang/Tooling/Core/QualTypeNames.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/GlobalDecl.h"
 #include "clang/AST/Mangle.h"
+#include "clang/AST/QualTypeNames.h"
 
 #include 
 #include 
 
 namespace clang {
 
 namespace TypeName {
-/// \brief Generates a QualType that can be used to name the same type
-/// if used at the end of the current translation unit. This ignores
-/// issues such as type shadowing.
-///
-/// \param[in] QT - the type for which the fully qualified type will be
-/// returned.
-/// \param[in] Ctx - the ASTContext to be used.
-/// \param[in] WithGlobalNsPrefix - Indicate whether the global namespace
-/// specifier "::" should be prepended or not.
-static QualType getFullyQualifiedType(QualType QT, const ASTContext &Ctx,
-  bool WithGlobalNsPrefix);
 
 /// \brief Create a NestedNameSpecifier for Namesp and its enclosing
 /// scopes.
Index: lib/AST/CMakeLists.txt
===
--- lib/AST/CMakeLists.txt
+++ lib/AST/CMakeLists.txt
@@ -49,6 +49,7 @@
   ODRHash.cpp
   OpenMPClause.cpp
   ParentMap.cpp
+  QualTypeNames.cpp
   RawCommentList.cpp
   RecordLayout.cpp
   RecordLayoutBuilder.cpp
Index: include/clang/AST/QualTypeNames.h
===
--- include/clang/AST/QualTypeNames.h
+++ include/clang/AST/QualTypeNames.h
@@ -56,8 +56,8 @@
 //
 // ===--===//
 
-#ifndef LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
-#define LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
+#ifndef LLVM_CLANG_AST_QUALTYPENAMES_H
+#define LLVM_CLANG_AST_QUALTYPENAMES_H
 
 #include "clang/AST/ASTContext.h"
 
@@ -71,9 +71,20 @@
 /// \param[in] Ctx - the ASTContext to be used.
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
-std::string getFullyQualifiedName(QualType QT,
-  const ASTContext &Ctx,
+std::string getFullyQualifiedName(QualType QT, const ASTContext &Ctx,
   bool WithGlobalNsPrefix = false);
-}  // end namespace TypeName
-}  // end namespace clang
-#endif  // LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
+
+/// \brief Generates a QualType that can be used to name the same type
+/// if used at the end of the current translation unit. This ignores
+/// issues such as type shadowing.
+///
+/// \param[in] QT - the type for which the fully qualified type will be
+/// returned.
+/// \param[in] Ctx - the ASTContext to be used.
+/// \param[in] WithGlobalNsPrefix - Indicate whether the global namespace
+/// specifier "::" should be prepended or not.
+QualType getFullyQualifiedType(QualType QT, const ASTContext &Ctx,
+   bool WithGlobalNsPrefix = false);
+} // end namespace TypeName
+} // end namespace clang
+#endif // LLVM_CLANG_TOOLING_CORE_QUALTYPENAMES_H
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 120016.
ilya-biryukov added a comment.

- Extracted the move of QualTypeNames to AST into a separate revision.


https://reviews.llvm.org/D38538

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/call.cpp
  test/CodeCompletion/qualifiers-as-written.cpp
  test/CodeCompletion/uninstantiated_params.cpp
  test/Index/complete-cxx-inline-methods.cpp

Index: test/Index/complete-cxx-inline-methods.cpp
===
--- test/Index/complete-cxx-inline-methods.cpp
+++ test/Index/complete-cxx-inline-methods.cpp
@@ -25,7 +25,7 @@
 
 // RUN: c-index-test -code-completion-at=%s:4:9 -std=c++98 %s | FileCheck %s
 // RUN: c-index-test -code-completion-at=%s:13:7 -std=c++98 %s | FileCheck %s
-// CHECK:  CXXMethod:{ResultType MyCls::Vec &}{TypedText operator=}{LeftParen (}{Placeholder const MyCls::Vec &}{RightParen )} (79)
+// CHECK:  CXXMethod:{ResultType Vec &}{TypedText operator=}{LeftParen (}{Placeholder const Vec &}{RightParen )} (79)
 // CHECK-NEXT: StructDecl:{TypedText Vec}{Text ::} (75)
 // CHECK-NEXT: FieldDecl:{ResultType int}{TypedText x} (35)
 // CHECK-NEXT: FieldDecl:{ResultType int}{TypedText y} (35)
Index: test/CodeCompletion/uninstantiated_params.cpp
===
--- test/CodeCompletion/uninstantiated_params.cpp
+++ test/CodeCompletion/uninstantiated_params.cpp
@@ -9,5 +9,5 @@
   unique_ptr x;
   x.
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:5 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
-  // CHECK-CC1: [#void#]reset({#<#unique_ptr::pointer ptr = pointer()#>#})
+  // CHECK-CC1: [#void#]reset({#<#pointer ptr = pointer()#>#})
 }
Index: test/CodeCompletion/qualifiers-as-written.cpp
===
--- /dev/null
+++ test/CodeCompletion/qualifiers-as-written.cpp
@@ -0,0 +1,30 @@
+struct foo {
+  typedef int type;
+
+  type method(type, foo::type, ::foo::type, ::foo::foo::type);
+};
+
+namespace ns {
+  struct bar {
+  };
+
+  struct baz {
+  };
+
+  int func(foo::type a, bar b, baz c);
+}
+
+typedef ns::bar bar;
+
+int func(foo a, bar b, ns::bar c, ns::baz d);
+using ns::func;
+
+void test() {
+  foo().method(0, 0, 0, 0);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:9 %s -o - | FileCheck %s --check-prefix=CHECK-1
+  // CHECK-1: COMPLETION: method : [#type#]method(<#type#>, <#foo::type#>, <#::foo::type#>, <#::foo::foo::type#>)
+  f
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:26:3 %s -o - | FileCheck %s --check-prefix=CHECK-2
+  // CHECK-2: COMPLETION: func : [#int#]func(<#foo a#>, <#bar b#>, <#ns::bar c#>, <#ns::baz d#>
+  // CHECK-2: COMPLETION: func : [#int#]func(<#foo::type a#>, <#bar b#>, <#baz c#>
+}
Index: test/CodeCompletion/call.cpp
===
--- test/CodeCompletion/call.cpp
+++ test/CodeCompletion/call.cpp
@@ -19,10 +19,10 @@
   f(Y(), 0, 0);
   // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>)
-  // CHECK-CC1: f(N::Y y, <#int ZZ#>)
+  // CHECK-CC1: f(Y y, <#int ZZ#>)
   // CHECK-CC1-NEXT: f(int i, <#int j#>, int k)
   // CHECK-CC1-NEXT: f(float x, <#float y#>)
   // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
-  // CHECK-CC2-NOT: f(N::Y y, int ZZ)
+  // CHECK-CC2-NOT: f(Y y, int ZZ)
   // CHECK-CC2: f(int i, int j, <#int k#>)
 }
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/QualTypeNames.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
@@ -1495,6 +1496,7 @@
   Policy.AnonymousTagLocations = false;
   Policy.SuppressStrongLifetime = true;
   Policy.SuppressUnwrittenScope = true;
+  Policy.SuppressScope = true;
   return Policy;
 }
 
@@ -2137,9 +2139,10 @@
   T = Method->getSendResultType(BaseType);
 else
   T = Method->getReturnType();
-  } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND))
+  } else if (const EnumConstantDecl *Enumerator = dyn_cast(ND)) {
 T = Context.getTypeDeclType(cast(Enumerator->getDeclContext()));
-  else if (isa(ND)) {
+T = clang::TypeName::getFullyQualifiedType(T, Context);
+  } else if (isa(ND)) {
 /* Do nothing: ignore unresolved using declarations*/
   } else if (const ObjCIvarDecl *Ivar = dyn_cast(ND)) {
 if (!BaseType.isNull())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The actual usage of `QualTypeNames` is in https://reviews.llvm.org/D38538.


https://reviews.llvm.org/D39224



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

[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai added a comment.

Dear Henry,

Sorry for my poor English, please Create a New patch or provide Chinese 
translation :)

Regards,
Leslie Zhai


Repository:
  rL LLVM

https://reviews.llvm.org/D31868



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

[PATCH] D38110: [libunwind][MIPS]: Add support for unwinding in O32 and N64 processes.

[John Baldwin via Phabricator via cfe-commits]

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

Ping?


https://reviews.llvm.org/D38110



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

r316424 - [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

[Peter Smith via cfe-commits]

Author: psmith
Date: Tue Oct 24 02:51:55 2017
New Revision: 316424

URL: http://llvm.org/viewvc/llvm-project?rev=316424&view=rev
Log:
[AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

When -mtune is used on AArch64 the -target-cpu is passed the value of the
cpu given to -mtune. As well as setting micro-architectural features of the
-mtune cpu, this will also add the architectural features such as support
for instructions. This can result in the backend using instructions that
are supported in the -mtune cpu but not supported in the target
architecture. For example use of the v8.1-a LSE extensions with -march=v8.

This change removes the setting of -target-cpu for -mtune, the -mcpu must
be used to set -target-cpu. This has the effect of removing all non-hard
coded benefits of mtune but it does produce correct output when -mtune cpu
with a later architecture than v8 is used.

Fixes PR34625

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



Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
cfe/trunk/test/Driver/aarch64-cpus.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp?rev=316424&r1=316423&r2=316424&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp Tue Oct 24 02:51:55 2017
@@ -20,14 +20,12 @@ using namespace clang;
 using namespace llvm::opt;
 
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
-/// targeting. Set \p A to the Arg corresponding to the -mcpu or -mtune
-/// arguments if they are provided, or to nullptr otherwise.
+/// targeting. Set \p A to the Arg corresponding to the -mcpu argument if it is
+/// provided, or to nullptr otherwise.
 std::string aarch64::getAArch64TargetCPU(const ArgList &Args, Arg *&A) {
   std::string CPU;
-  // If we have -mtune or -mcpu, use that.
-  if ((A = Args.getLastArg(clang::driver::options::OPT_mtune_EQ))) {
-CPU = StringRef(A->getValue()).lower();
-  } else if ((A = Args.getLastArg(options::OPT_mcpu_EQ))) {
+  // If we have -mcpu, use that.
+  if ((A = Args.getLastArg(options::OPT_mcpu_EQ))) {
 StringRef Mcpu = A->getValue();
 CPU = Mcpu.split("+").first.lower();
   }
@@ -122,6 +120,12 @@ getAArch64MicroArchFeaturesFromMtune(con
  const ArgList &Args,
  std::vector &Features) {
   std::string MtuneLowerCase = Mtune.lower();
+  // Check CPU name is valid
+  std::vector MtuneFeatures;
+  StringRef Tune;
+  if (!DecodeAArch64Mcpu(D, MtuneLowerCase, Tune, MtuneFeatures))
+return false;
+
   // Handle CPU name is 'native'.
   if (MtuneLowerCase == "native")
 MtuneLowerCase = llvm::sys::getHostCPUName();

Modified: cfe/trunk/test/Driver/aarch64-cpus.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-cpus.c?rev=316424&r1=316423&r2=316424&view=diff
==
--- cfe/trunk/test/Driver/aarch64-cpus.c (original)
+++ cfe/trunk/test/Driver/aarch64-cpus.c Tue Oct 24 02:51:55 2017
@@ -21,165 +21,188 @@
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck 
-check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck 
-check-prefix=CA35 %s
-// RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35 %s
+// RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck 
-check-prefix=CA35-TUNE %s
+// RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35-TUNE %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck 
-check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 
| FileCheck -check-prefix=ARM64-CA35 %s
-// RUN: %clang -target arm64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck 
-check-prefix=ARM64-CA35 %s
-// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 
| FileCheck -check-prefix=ARM64-CA35 %s
+// RUN: %clang -target arm64 -mtune=cortex-a35 -### -c %s 2>&1

[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

[Peter Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316424: [AArch64] Fix PR34625 -mtune without -mcpu should 
not set -target-cpu (authored by psmith).

Changed prior to commit:
  https://reviews.llvm.org/D39179?vs=119837&id=120025#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39179

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
  cfe/trunk/test/Driver/aarch64-cpus.c

Index: cfe/trunk/test/Driver/aarch64-cpus.c
===
--- cfe/trunk/test/Driver/aarch64-cpus.c
+++ cfe/trunk/test/Driver/aarch64-cpus.c
@@ -21,182 +21,205 @@
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
+// RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
+// RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
-// RUN: %clang -target arm64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
-// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
+// RUN: %clang -target arm64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35-TUNE %s
+// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35-TUNE %s
 // ARM64-CA35: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cortex-a35"
-
+// ARM64-CA35-TUNE: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
-// RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
+// RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
 // CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
+// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
-// RUN: %clang -target arm64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
-// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
+// RUN: %clang -target arm64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53-TUNE %s
+// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53-TUNE %s
 // ARM64-CA53: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cortex-a53"
+// ARM64-CA53-TUNE: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
-// RUN: %clang -target aarch64 -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s

[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D31868#904814, @MTC wrote:

> > One of the possible improvements for future work here would be to actually 
> > bind the second argument value to the buffer instead of just invalidating 
> > it. Like, after `memset(buf, 0, sizeof(buf))` the analyzer should know that 
> > all values in the `buf` array are `0`. In the analyzer we have the notion 
> > of *default bindings* to handle that (see documentation in 
> > docs/analyzer/RegionStore.txt for more details).
>
> `BindDefault()` is the only function that can make the default binding, is 
> it? If so, `evalMemset()` uses `bindDefault()`, the binding may not take 
> effect. Because the current `BindDefault()` logic is that if the memory area 
> has been initialized, then the default binding will no longer be done, see 
> https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Core/RegionStore.cpp#L429.
>  Before `evalMemset()`, `MallocMemAux()` in MallocChecker.cpp may have 
> already made the default binding. Am I right?


Seems so, and it also looks super counter-intuitive. We definitely do need to 
overwrite default bindings from time to time. When `RegionStore` itself wants 
to overwrite a default binding, it does `removeBindings()` first (see 
`invalidateGlobalRegion()` as an example). I think 
`ProgramState::bindDefault()` should behave this way as well, not sure about 
impact of such change.


Repository:
  rL LLVM

https://reviews.llvm.org/D31868



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

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: include/__libunwind_config.h:59-68
+# elif defined(__mips__) && defined(_ABIN32) && defined(__mips_soft_float)
+#  define _LIBUNWIND_TARGET_MIPS_N64 1
+#  define _LIBUNWIND_CONTEXT_SIZE 35
+#  define _LIBUNWIND_CURSOR_SIZE 46
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 66
 # elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
 #  define _LIBUNWIND_TARGET_MIPS_N64 1

Change the define of _LIBUNWIND_TARGET_MIPS_N64 to 
_LIBUNWIND_TARGET_MIPS_NEWABI, then these two elif branches can have the 
condition (defined(_ABIN32) && defined(_ABI64) and refactored into one elif 
branch.



Comment at: include/__libunwind_config.h:62
+#  define _LIBUNWIND_CONTEXT_SIZE 35
+#  define _LIBUNWIND_CURSOR_SIZE 46
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 66

Shouldn't this 46 be 47?



Comment at: src/AddressSpace.hpp:201
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if defined(__LP64__) || defined(__mips_n32)
+  return get64(addr);

   defined(__mips_n32) -> (defined(__mips__) && defined(_ABIN32))



Comment at: src/UnwindRegistersRestore.S:548
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) && 
defined(__mips_soft_float)
 

This line is overly long, break with '\' after the second &&.



Comment at: src/UnwindRegistersSave.S:146
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) && 
defined(__mips_soft_float)
 

This line looks overly long, break before the second &&.



Comment at: src/libunwind.cpp:63-66
+#elif defined(__mips__) && defined(_ABIN32) && defined(__mips_soft_float)
+# define REGISTER_KIND Registers_mips_n64
 #elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
 # define REGISTER_KIND Registers_mips_n64

Fold these two branches together and have the condition (defined(_ABIN32) || 
defined(_ABI64), then follow-up by renaming Registers_mips_n64 to 
Register_mips_NEWABI. Add a comment to the definition of that class stating 
that it covers both n32 and n64.


https://reviews.llvm.org/D39074



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

Re: r316268 - [Sema] Fixes for enum handling for tautological comparison diagnostics

[Hans Wennborg via cfe-commits]

On Mon, Oct 23, 2017 at 2:02 PM, Roman Lebedev  wrote:
> On Mon, Oct 23, 2017 at 2:13 PM, Hans Wennborg  wrote:
> Hi.
>
>> This seems to have had the side effect of introducing a new warning in
>> Chromium builds:
>>
>> ../../third_party/expat/files/lib/xmlparse.c(2429,24):  error:
>> comparison of integers of different signs: 'enum XML_Error' and
>> 'unsigned int' [-Werror,-Wsign-compare]
>>   if (code > 0 && code < sizeof(message)/sizeof(message[0]))
>>    ^ ~~
> (I guess this is on windows)

Yes.

>
>> I'm not sure if this was intentional or not.
>>
>> The warning seems technically correct here, though not very useful in
>> this specific case.
> I *believe* that was caused by the fix for IntRange::forValueOfCanonicalType()
> to use enum's underlying type for C code. That fix was absolutely intentional.
>
> However that new -Wsign-compare diagnostic (and i suspect there may be
> more repercussions) was not really intentional.
> However as you have said, and i think i agree, the diagnostic valid.
>
> So perhaps i simply should add a test and release notes entry?

Sounds reasonable. It turns out we only got this single extra
-Wsign-compare warning, so it didn't turn out to be a big problem for
us.

>
> Roman.
>
>> On Sat, Oct 21, 2017 at 6:44 PM, Roman Lebedev via cfe-commits
>>  wrote:
>>> Author: lebedevri
>>> Date: Sat Oct 21 09:44:03 2017
>>> New Revision: 316268
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=316268&view=rev
>>> Log:
>>> [Sema] Fixes for enum handling for tautological comparison diagnostics
>>>
>>> Summary:
>>> As Mattias Eriksson has reported in PR35009, in C, for enums, the 
>>> underlying type should
>>> be used when checking for the tautological comparison, unlike C++, where 
>>> the enumerator
>>> values define the value range. So if not in CPlusPlus mode, use the enum 
>>> underlying type.
>>>
>>> Also, i have discovered a problem (a crash) when evaluating 
>>> tautological-ness of the following comparison:
>>> ```
>>> enum A { A_a = 0 };
>>> if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 
>>> is always false}}
>>> return 0;
>>> ```
>>> This affects both the C and C++, but after the first fix, only C++ code was 
>>> affected.
>>> That was also fixed, while preserving (i think?) the proper diagnostic 
>>> output.
>>>
>>> And while there, attempt to enhance the test coverage.
>>> Yes, some tests got moved around, sorry about that :)
>>>
>>> Fixes PR35009
>>>
>>> Reviewers: aaron.ballman, rsmith, rjmccall
>>>
>>> Reviewed By: aaron.ballman
>>>
>>> Subscribers: Rakete, efriedma, materi, cfe-commits
>>>
>>> Tags: #clang
>>>
>>> Differential Revision: https://reviews.llvm.org/D39122
>>>
>>> Added:
>>> cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
>>> cfe/trunk/test/Sema/tautological-constant-enum-compare.c
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaChecking.cpp
>>> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
>>> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=316268&r1=316267&r2=316268&view=diff
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sat Oct 21 09:44:03 2017
>>> @@ -8181,8 +8181,12 @@ struct IntRange {
>>>  if (const AtomicType *AT = dyn_cast(T))
>>>T = AT->getValueType().getTypePtr();
>>>
>>> -// For enum types, use the known bit width of the enumerators.
>>> -if (const EnumType *ET = dyn_cast(T)) {
>>> +if (!C.getLangOpts().CPlusPlus) {
>>> +  // For enum types in C code, use the underlying datatype.
>>> +  if (const EnumType *ET = dyn_cast(T))
>>> +T = 
>>> ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
>>> +} else if (const EnumType *ET = dyn_cast(T)) {
>>> +  // For enum types in C++, use the known bit width of the enumerators.
>>>EnumDecl *Enum = ET->getDecl();
>>>// In C++11, enums without definitions can have an explicitly 
>>> specified
>>>// underlying type.  Use this type to compute the range.
>>> @@ -8584,8 +8588,10 @@ bool isNonBooleanUnsignedValue(Expr *E)
>>>  }
>>>
>>>  enum class LimitType {
>>> -  Max, // e.g. 32767 for short
>>> -  Min  // e.g. -32768 for short
>>> +  Max = 1U << 0U,  // e.g. 32767 for short
>>> +  Min = 1U << 1U,  // e.g. -32768 for short
>>> +  Both = Max | Min // When the value is both the Min and the Max limit at 
>>> the
>>> +   // same time; e.g. in C++, A::a in enum A { a = 0 };
>>>  };
>>>
>>>  /// Checks whether Expr 'Constant' may be the
>>> @@ -8608,6 +8614,10 @@ llvm::Optional IsTypeLimit(Se
>>>
>>>IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>>>
>>> +  // Special-case for C++

r316433 - [Tooling] Add a factory method for CommonOptionsParser

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Oct 24 06:10:58 2017
New Revision: 316433

URL: http://llvm.org/viewvc/llvm-project?rev=316433&view=rev
Log:
[Tooling] Add a factory method for CommonOptionsParser

Summary: This returns error instead of exiting the program in case of error.

Reviewers: klimek, hokein

Reviewed By: hokein

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
cfe/trunk/lib/Tooling/CommonOptionsParser.cpp

Modified: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CommonOptionsParser.h?rev=316433&r1=316432&r2=316433&view=diff
==
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h (original)
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h Tue Oct 24 06:10:58 
2017
@@ -30,6 +30,7 @@
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
 
 namespace clang {
 namespace tooling {
@@ -86,13 +87,18 @@ public:
   /// All options not belonging to \p Category become hidden.
   ///
   /// It also allows calls to set the required number of positional parameters.
-  ///
-  /// This constructor exits program in case of error.
   CommonOptionsParser(int &argc, const char **argv,
   llvm::cl::OptionCategory &Category,
   llvm::cl::NumOccurrencesFlag OccurrencesFlag,
   const char *Overview = nullptr);
 
+  /// \brief A factory method that is similar to the above constructor, except
+  /// this returns an error instead exiting the program on error.
+  static llvm::Expected
+  create(int &argc, const char **argv, llvm::cl::OptionCategory &Category,
+ llvm::cl::NumOccurrencesFlag OccurrencesFlag,
+ const char *Overview = nullptr);
+
   /// Returns a reference to the loaded compilations database.
   CompilationDatabase &getCompilations() {
 return *Compilations;
@@ -106,6 +112,13 @@ public:
   static const char *const HelpMessage;
 
 private:
+  CommonOptionsParser() = default;
+
+  llvm::Error init(int &argc, const char **argv,
+   llvm::cl::OptionCategory &Category,
+   llvm::cl::NumOccurrencesFlag OccurrencesFlag,
+   const char *Overview);
+
   std::unique_ptr Compilations;
   std::vector SourcePathList;
 };

Modified: cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CommonOptionsParser.cpp?rev=316433&r1=316432&r2=316433&view=diff
==
--- cfe/trunk/lib/Tooling/CommonOptionsParser.cpp (original)
+++ cfe/trunk/lib/Tooling/CommonOptionsParser.cpp Tue Oct 24 06:10:58 2017
@@ -24,9 +24,9 @@
 //
 
//===--===//
 
-#include "llvm/Support/CommandLine.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -81,7 +81,7 @@ std::vector ArgumentsAdj
   return Commands;
 }
 
-CommonOptionsParser::CommonOptionsParser(
+llvm::Error CommonOptionsParser::init(
 int &argc, const char **argv, cl::OptionCategory &Category,
 llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
   static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden,
@@ -105,20 +105,30 @@ CommonOptionsParser::CommonOptionsParser
   cl::desc("Additional argument to prepend to the compiler command line"),
   cl::cat(Category), cl::sub(*cl::AllSubCommands));
 
+  cl::ResetAllOptionOccurrences();
+
   cl::HideUnrelatedOptions(Category);
 
   std::string ErrorMessage;
   Compilations =
   FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage);
-  if (!Compilations && !ErrorMessage.empty())
-llvm::errs() << ErrorMessage;
-  cl::ParseCommandLineOptions(argc, argv, Overview);
+  if (!ErrorMessage.empty())
+ErrorMessage.append("\n");
+  llvm::raw_string_ostream OS(ErrorMessage);
+  // Stop initializing if command-line option parsing failed.
+  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+OS.flush();
+return llvm::make_error("[CommonOptionsParser]: " +
+   ErrorMessage,
+   llvm::inconvertibleErrorCode());
+  }
+
   cl::PrintOptionValues();
 
   SourcePathList = SourcePaths;
   if ((OccurrencesFlag == cl::ZeroOrMore || OccurrencesFlag == cl::Optional) &&
   SourcePathList.empty())
-return;
+return llvm::Error::success();
   if (!Compilations) {
 if (!BuildPath.empty()) {
   Compilations =
@@ -142,4 +152,27 @@ CommonOptionsParser::CommonOption

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316433: [Tooling] Add a factory method for 
CommonOptionsParser (authored by ioeric).

Repository:
  rL LLVM

https://reviews.llvm.org/D39042

Files:
  cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
  cfe/trunk/lib/Tooling/CommonOptionsParser.cpp

Index: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
===
--- cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
+++ cfe/trunk/include/clang/Tooling/CommonOptionsParser.h
@@ -30,6 +30,7 @@
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
 
 namespace clang {
 namespace tooling {
@@ -86,13 +87,18 @@
   /// All options not belonging to \p Category become hidden.
   ///
   /// It also allows calls to set the required number of positional parameters.
-  ///
-  /// This constructor exits program in case of error.
   CommonOptionsParser(int &argc, const char **argv,
   llvm::cl::OptionCategory &Category,
   llvm::cl::NumOccurrencesFlag OccurrencesFlag,
   const char *Overview = nullptr);
 
+  /// \brief A factory method that is similar to the above constructor, except
+  /// this returns an error instead exiting the program on error.
+  static llvm::Expected
+  create(int &argc, const char **argv, llvm::cl::OptionCategory &Category,
+ llvm::cl::NumOccurrencesFlag OccurrencesFlag,
+ const char *Overview = nullptr);
+
   /// Returns a reference to the loaded compilations database.
   CompilationDatabase &getCompilations() {
 return *Compilations;
@@ -106,6 +112,13 @@
   static const char *const HelpMessage;
 
 private:
+  CommonOptionsParser() = default;
+
+  llvm::Error init(int &argc, const char **argv,
+   llvm::cl::OptionCategory &Category,
+   llvm::cl::NumOccurrencesFlag OccurrencesFlag,
+   const char *Overview);
+
   std::unique_ptr Compilations;
   std::vector SourcePathList;
 };
Index: cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
===
--- cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
+++ cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
@@ -24,9 +24,9 @@
 //
 //===--===//
 
-#include "llvm/Support/CommandLine.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -81,7 +81,7 @@
   return Commands;
 }
 
-CommonOptionsParser::CommonOptionsParser(
+llvm::Error CommonOptionsParser::init(
 int &argc, const char **argv, cl::OptionCategory &Category,
 llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
   static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden,
@@ -105,20 +105,30 @@
   cl::desc("Additional argument to prepend to the compiler command line"),
   cl::cat(Category), cl::sub(*cl::AllSubCommands));
 
+  cl::ResetAllOptionOccurrences();
+
   cl::HideUnrelatedOptions(Category);
 
   std::string ErrorMessage;
   Compilations =
   FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage);
-  if (!Compilations && !ErrorMessage.empty())
-llvm::errs() << ErrorMessage;
-  cl::ParseCommandLineOptions(argc, argv, Overview);
+  if (!ErrorMessage.empty())
+ErrorMessage.append("\n");
+  llvm::raw_string_ostream OS(ErrorMessage);
+  // Stop initializing if command-line option parsing failed.
+  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+OS.flush();
+return llvm::make_error("[CommonOptionsParser]: " +
+   ErrorMessage,
+   llvm::inconvertibleErrorCode());
+  }
+
   cl::PrintOptionValues();
 
   SourcePathList = SourcePaths;
   if ((OccurrencesFlag == cl::ZeroOrMore || OccurrencesFlag == cl::Optional) &&
   SourcePathList.empty())
-return;
+return llvm::Error::success();
   if (!Compilations) {
 if (!BuildPath.empty()) {
   Compilations =
@@ -142,4 +152,27 @@
   AdjustingCompilations->appendArgumentsAdjuster(
   getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
   Compilations = std::move(AdjustingCompilations);
+  return llvm::Error::success();
+}
+
+llvm::Expected CommonOptionsParser::create(
+int &argc, const char **argv, llvm::cl::OptionCategory &Category,
+llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
+  CommonOptionsParser Parser;
+  llvm::Error Err =
+  Parser.init(argc, argv, Category, OccurrencesFlag, Overview);
+  if (Err)
+return std::move(Err);
+  return std::move(Parser);
+}
+
+CommonOptionsParser::CommonOptionsParser(
+in

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso created this revision.

An unscoped enumeration used as template argument, should not
have any qualified information about its enclosing scope, as
its visibility is global.

In the case of scoped enumerations, they must include information
about their enclosing scope.

For the below test:

  enum unscoped { a };
  enum class scoped { b };
  
  template struct tmpl_u { };
  template struct tmpl_s { };
  
  tmpl_u tmpl_unscoped;
  tmpl_s tmpl_scoped;

gives the following encoded names:

  DW_TAG_structure_type "tmpl_u"
  DW_TAG_structure_type "tmpl_s"

The incorrectly qualified name causes a debugger to show qualified
names for both scoped and unscoped enumerations.


https://reviews.llvm.org/D39239

Files:
  lib/AST/Decl.cpp
  test/Modules/odr.cpp
  test/SemaCXX/return-noreturn.cpp
  test/SemaTemplate/temp_arg_enum_printing.cpp
  test/SemaTemplate/temp_arg_enum_printing_more.cpp
  unittests/AST/NamedDeclPrinterTest.cpp

Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -143,7 +143,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "enum X { A };",
 "A",
-"X::A"));
+"A"));
 }
 
 TEST(NamedDeclPrinter, TestScopedNamedEnum) {
@@ -164,7 +164,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "class X { enum Y { A }; };",
 "A",
-"X::Y::A"));
+"X::A"));
 }
 
 TEST(NamedDeclPrinter, TestClassWithScopedNamedEnum) {
Index: test/SemaTemplate/temp_arg_enum_printing_more.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing_more.cpp
+++ test/SemaTemplate/temp_arg_enum_printing_more.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -ast-print %s -std=c++11 | FileCheck %s
+
+// Make sure that for template value arguments that are unscoped enumerators,
+// no qualified enum information is included in their name, as their visibility
+// is global. In the case of scoped enumerators, they must include information
+// about their enum enclosing scope.
+
+enum E1 { e1 };
+template struct tmpl_1 {};
+// CHECK: template<> struct tmpl_1
+tmpl_1 TMPL_1;  // Name must be 'e1'.
+
+namespace nsp_1 { enum E2 { e2 }; }
+template struct tmpl_2 {};
+// CHECK: template<> struct tmpl_2
+tmpl_2 TMPL_2;   // Name must be 'nsp_1::e2'.
+
+enum class E3 { e3 };
+template struct tmpl_3 {};
+// CHECK: template<> struct tmpl_3
+tmpl_3 TMPL_3;  // Name must be 'E3::e3'.
+
+namespace nsp_2 { enum class E4 { e4 }; }
+template struct tmpl_4 {};
+// CHECK: template<> struct tmpl_4
+tmpl_4 TMPL_4;   // Name must be 'nsp_2::E4::e4'.
Index: test/SemaTemplate/temp_arg_enum_printing.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing.cpp
+++ test/SemaTemplate/temp_arg_enum_printing.cpp
@@ -13,9 +13,9 @@
 void foo();
   
 void test() {
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo();
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo<(NamedEnum)1>();
   // CHECK: template<> void foo<2>()
   NamedEnumNS::foo<(NamedEnum)2>();
Index: test/SemaCXX/return-noreturn.cpp
===
--- test/SemaCXX/return-noreturn.cpp
+++ test/SemaCXX/return-noreturn.cpp
@@ -143,7 +143,7 @@
 } // expected-warning {{control reaches end of non-void function}}
 
 void PR9412_f() {
-PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
+PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
 }
 
 struct NoReturn {
Index: test/Modules/odr.cpp
===
--- test/Modules/odr.cpp
+++ test/Modules/odr.cpp
@@ -18,6 +18,6 @@
 // expected-note@a.h:3 {{declaration of 'f' does not match}}
 // expected-note@a.h:1 {{definition has no member 'm'}}
 
-// expected-error@b.h:5 {{'E::e2' from module 'b' is not present in definition of 'E' in module 'a'}}
+// expected-error@b.h:5 {{'e2' from module 'b' is not present in definition of 'E' in module 'a'}}
 // expected-error@b.h:3 {{'Y::f' from module 'b' is not present in definition of 'Y' in module 'a'}}
 // expected-error@b.h:2 {{'Y::m' from module 'b' is not present in definition of 'Y' in module 'a'}}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1514,7 +1514,10 @@
   // enumerator is declared in the scope that immediately contains
   // the enum-specifier. Each scoped enumerator is declared in the
   // scope of the enumeration.
-  if (ED->isScoped() || ED->getIdentifier())
+  // For the case of unscoped enumerator, do

r316436 - Do not add a colon chunk to the code completion of class inheritance access modifiers

[Erik Verbruggen via cfe-commits]

Author: erikjv
Date: Tue Oct 24 06:46:58 2017
New Revision: 316436

URL: http://llvm.org/viewvc/llvm-project?rev=316436&view=rev
Log:
Do not add a colon chunk to the code completion of class inheritance access 
modifiers

With enabled CINDEXTEST_CODE_COMPLETE_PATTERNS env option (which enables
IncludeCodePatterns in completion options) code completion after colon
currently suggests access modifiers with 2 completion chunks which is
incorrect.

Example:
class A : B
{
}

Currently we get 'NotImplemented:{TypedText public}{Colon :} (40)'
but the correct line is just 'NotImplemented:{TypedText public} (40)'

The fix introduces more specific scope that occurs between ':' and '{'
It allows us to determine when we don't need to add ':' as a second
chunk to the public/protected/private access modifiers.

Patch by Ivan Donchevskii!

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


Modified:
cfe/trunk/include/clang/Sema/Scope.h
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/Index/complete-super.cpp

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=316436&r1=316435&r2=316436&view=diff
==
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Tue Oct 24 06:46:58 2017
@@ -127,6 +127,9 @@ public:
 
 /// This is a compound statement scope.
 CompoundStmtScope = 0x40,
+
+/// We are between inheritance colon and the real class/struct definition 
scope.
+ClassInheritanceScope = 0x80,
   };
 private:
   /// The parent scope for this scope.  This is null for the translation-unit

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=316436&r1=316435&r2=316436&view=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Tue Oct 24 06:46:58 2017
@@ -3195,6 +3195,9 @@ void Parser::ParseCXXMemberSpecification
   }
 
   if (Tok.is(tok::colon)) {
+ParseScope InheritanceScope(this, getCurScope()->getFlags() |
+  Scope::ClassInheritanceScope);
+
 ParseBaseClause(TagDecl);
 if (!Tok.is(tok::l_brace)) {
   bool SuggestFixIt = false;

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=316436&r1=316435&r2=316436&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Tue Oct 24 06:46:58 2017
@@ -1658,21 +1658,23 @@ static void AddOrdinaryNameResults(Sema:
   if (CCC == Sema::PCC_Class) {
 AddTypedefResult(Results);
 
+bool IsNotInheritanceScope =
+!(S->getFlags() & Scope::ClassInheritanceScope);
 // public:
 Builder.AddTypedTextChunk("public");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // protected:
 Builder.AddTypedTextChunk("protected");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // private:
 Builder.AddTypedTextChunk("private");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
   }

Modified: cfe/trunk/test/Index/complete-super.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-super.cpp?rev=316436&r1=316435&r2=316436&view=diff
==
--- cfe/trunk/test/Index/complete-super.cpp (original)
+++ cfe/trunk/test/Index/complete-super.cpp Tue Oct 24 06:46:58 2017
@@ -40,3 +40,8 @@ void B::bar(float real) {
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test 
-code-completion-at=%s:10:12 %s | FileCheck 
-check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

[Erik Verbruggen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316436: Do not add a colon chunk to the code completion of 
class inheritance access… (authored by erikjv).

Changed prior to commit:
  https://reviews.llvm.org/D38618?vs=119273&id=120067#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38618

Files:
  cfe/trunk/include/clang/Sema/Scope.h
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/Index/complete-super.cpp


Index: cfe/trunk/include/clang/Sema/Scope.h
===
--- cfe/trunk/include/clang/Sema/Scope.h
+++ cfe/trunk/include/clang/Sema/Scope.h
@@ -127,6 +127,9 @@
 
 /// This is a compound statement scope.
 CompoundStmtScope = 0x40,
+
+/// We are between inheritance colon and the real class/struct definition 
scope.
+ClassInheritanceScope = 0x80,
   };
 private:
   /// The parent scope for this scope.  This is null for the translation-unit
Index: cfe/trunk/test/Index/complete-super.cpp
===
--- cfe/trunk/test/Index/complete-super.cpp
+++ cfe/trunk/test/Index/complete-super.cpp
@@ -40,3 +40,8 @@
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test 
-code-completion-at=%s:10:12 %s | FileCheck 
-check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText public} (40)
Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -1658,21 +1658,23 @@
   if (CCC == Sema::PCC_Class) {
 AddTypedefResult(Results);
 
+bool IsNotInheritanceScope =
+!(S->getFlags() & Scope::ClassInheritanceScope);
 // public:
 Builder.AddTypedTextChunk("public");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // protected:
 Builder.AddTypedTextChunk("protected");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
 
 // private:
 Builder.AddTypedTextChunk("private");
-if (Results.includeCodePatterns())
+if (IsNotInheritanceScope && Results.includeCodePatterns())
   Builder.AddChunk(CodeCompletionString::CK_Colon);
 Results.AddResult(Result(Builder.TakeString()));
   }
Index: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp
@@ -3195,6 +3195,9 @@
   }
 
   if (Tok.is(tok::colon)) {
+ParseScope InheritanceScope(this, getCurScope()->getFlags() |
+  Scope::ClassInheritanceScope);
+
 ParseBaseClause(TagDecl);
 if (!Tok.is(tok::l_brace)) {
   bool SuggestFixIt = false;


Index: cfe/trunk/include/clang/Sema/Scope.h
===
--- cfe/trunk/include/clang/Sema/Scope.h
+++ cfe/trunk/include/clang/Sema/Scope.h
@@ -127,6 +127,9 @@
 
 /// This is a compound statement scope.
 CompoundStmtScope = 0x40,
+
+/// We are between inheritance colon and the real class/struct definition scope.
+ClassInheritanceScope = 0x80,
   };
 private:
   /// The parent scope for this scope.  This is null for the translation-unit
Index: cfe/trunk/test/Index/complete-super.cpp
===
--- cfe/trunk/test/Index/complete-super.cpp
+++ cfe/trunk/test/Index/complete-super.cpp
@@ -40,3 +40,8 @@
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText private}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText protected}{Colon :} (40)
 // CHECK-ACCESS-PATTERN: NotImplemented:{TypedText public}{Colon :} (40)
+
+// RUN: env CINDEXTEST_CODE_COMPLETE_PATTERNS=1 c-index-test -code-completion-at=%s:10:12 %s | FileCheck -check-prefix=CHECK-INHERITANCE-PATTERN %s
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText private} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText protected} (40)
+// CHECK-INHERITANCE-PATTERN: NotImplemented:{TypedText public} (40)
Index: cfe

[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso updated this revision to Diff 120066.

https://reviews.llvm.org/D39239

Files:
  lib/AST/Decl.cpp
  test/Modules/odr.cpp
  test/SemaCXX/return-noreturn.cpp
  test/SemaTemplate/temp_arg_enum_printing.cpp
  test/SemaTemplate/temp_arg_enum_printing_more.cpp
  unittests/AST/NamedDeclPrinterTest.cpp

Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -143,7 +143,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "enum X { A };",
 "A",
-"X::A"));
+"A"));
 }
 
 TEST(NamedDeclPrinter, TestScopedNamedEnum) {
@@ -164,7 +164,7 @@
   ASSERT_TRUE(PrintedWrittenNamedDeclCXX11Matches(
 "class X { enum Y { A }; };",
 "A",
-"X::Y::A"));
+"X::A"));
 }
 
 TEST(NamedDeclPrinter, TestClassWithScopedNamedEnum) {
Index: test/SemaTemplate/temp_arg_enum_printing_more.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing_more.cpp
+++ test/SemaTemplate/temp_arg_enum_printing_more.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -ast-print %s -std=c++11 | FileCheck %s
+
+// Make sure that for template value arguments that are unscoped enumerators,
+// no qualified enum information is included in their name, as their visibility
+// is global. In the case of scoped enumerators, they must include information
+// about their enum enclosing scope.
+
+enum E1 { e1 };
+template struct tmpl_1 {};
+// CHECK: template<> struct tmpl_1
+tmpl_1 TMPL_1;  // Name must be 'e1'.
+
+namespace nsp_1 { enum E2 { e2 }; }
+template struct tmpl_2 {};
+// CHECK: template<> struct tmpl_2
+tmpl_2 TMPL_2;   // Name must be 'nsp_1::e2'.
+
+enum class E3 { e3 };
+template struct tmpl_3 {};
+// CHECK: template<> struct tmpl_3
+tmpl_3 TMPL_3;  // Name must be 'E3::e3'.
+
+namespace nsp_2 { enum class E4 { e4 }; }
+template struct tmpl_4 {};
+// CHECK: template<> struct tmpl_4
+tmpl_4 TMPL_4;   // Name must be 'nsp_2::E4::e4'.
Index: test/SemaTemplate/temp_arg_enum_printing.cpp
===
--- test/SemaTemplate/temp_arg_enum_printing.cpp
+++ test/SemaTemplate/temp_arg_enum_printing.cpp
@@ -13,9 +13,9 @@
 void foo();
   
 void test() {
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo();
-  // CHECK: template<> void foo()
+  // CHECK: template<> void foo()
   NamedEnumNS::foo<(NamedEnum)1>();
   // CHECK: template<> void foo<2>()
   NamedEnumNS::foo<(NamedEnum)2>();
Index: test/SemaCXX/return-noreturn.cpp
===
--- test/SemaCXX/return-noreturn.cpp
+++ test/SemaCXX/return-noreturn.cpp
@@ -143,7 +143,7 @@
 } // expected-warning {{control reaches end of non-void function}}
 
 void PR9412_f() {
-PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
+PR9412_t(); // expected-note {{in instantiation of function template specialization 'PR9412_t' requested here}}
 }
 
 struct NoReturn {
Index: test/Modules/odr.cpp
===
--- test/Modules/odr.cpp
+++ test/Modules/odr.cpp
@@ -18,6 +18,6 @@
 // expected-note@a.h:3 {{declaration of 'f' does not match}}
 // expected-note@a.h:1 {{definition has no member 'm'}}
 
-// expected-error@b.h:5 {{'E::e2' from module 'b' is not present in definition of 'E' in module 'a'}}
+// expected-error@b.h:5 {{'e2' from module 'b' is not present in definition of 'E' in module 'a'}}
 // expected-error@b.h:3 {{'Y::f' from module 'b' is not present in definition of 'Y' in module 'a'}}
 // expected-error@b.h:2 {{'Y::m' from module 'b' is not present in definition of 'Y' in module 'a'}}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1514,7 +1514,10 @@
   // enumerator is declared in the scope that immediately contains
   // the enum-specifier. Each scoped enumerator is declared in the
   // scope of the enumeration.
-  if (ED->isScoped() || ED->getIdentifier())
+  // For the case of unscoped enumerator, do not include in the qualified
+  // name any information about its enum enclosing scope, as is visibility
+  // is global.
+  if (ED->isScoped())
 OS << *ED;
   else
 continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316437 - Add Forgotten test for: Fix template parameter default args missed if redecled

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Oct 24 06:51:07 2017
New Revision: 316437

URL: http://llvm.org/viewvc/llvm-project?rev=316437&view=rev
Log:
Add Forgotten test for: Fix template parameter default args missed if redecled

Addendum to differential revision: https://reviews.llvm.org/D39127

Added:
cfe/trunk/test/SemaCXX/template-default-param-through-using.cpp

Added: cfe/trunk/test/SemaCXX/template-default-param-through-using.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/template-default-param-through-using.cpp?rev=316437&view=auto
==
--- cfe/trunk/test/SemaCXX/template-default-param-through-using.cpp (added)
+++ cfe/trunk/test/SemaCXX/template-default-param-through-using.cpp Tue Oct 24 
06:51:07 2017
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+namespace llvm {
+  template struct StringSet;
+  template struct Int;
+  template  class Outer>
+struct TemplTempl;
+}
+
+namespace lld {
+  using llvm::StringSet;
+  using llvm::Int;
+  using llvm::TemplTempl;
+};
+
+namespace llvm {
+  template struct StringSet;
+}
+
+template struct Temp{};
+
+namespace llvm {
+  template struct StringSet{};
+  template struct Int{};
+  template  class Outer = Temp>
+struct TemplTempl{};
+};
+
+namespace lld {
+  StringSet<> s;
+  Int<> i;
+  TemplTempl tt;
+}


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

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 120068.
ioeric added a comment.

- Experimented and refined the interfaces. o Get rid of `ExecutionConfig` 
class; pass actions into executors directly. o Add ArgumentAdjuster to 
ExecutionContext. o Make command-line based executor selection more explicit by 
requiring a "--executor" option.


https://reviews.llvm.org/D34272

Files:
  include/clang/Tooling/CommonOptionsParser.h
  include/clang/Tooling/Execution.h
  include/clang/Tooling/StandaloneExecution.h
  include/clang/Tooling/ToolExecutorPluginRegistry.h
  include/clang/Tooling/Tooling.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/CommonOptionsParser.cpp
  lib/Tooling/Execution.cpp
  lib/Tooling/StandaloneExecution.cpp
  lib/Tooling/Tooling.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/ExecutionTest.cpp

Index: unittests/Tooling/ExecutionTest.cpp
===
--- /dev/null
+++ unittests/Tooling/ExecutionTest.cpp
@@ -0,0 +1,230 @@
+//===- unittest/Tooling/ExecutionTest.cpp - Tool execution tests. ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Execution.h"
+#include "clang/Tooling/StandaloneExecution.h"
+#include "clang/Tooling/ToolExecutorPluginRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+
+namespace {
+
+// This traverses the AST and outputs function name as key and "1" as value for
+// each function declaration.
+class ASTConsumerWithResult
+: public ASTConsumer,
+  public RecursiveASTVisitor {
+public:
+  using ASTVisitor = RecursiveASTVisitor;
+
+  explicit ASTConsumerWithResult(ExecutionContext *Context) : Context(Context) {
+assert(Context != nullptr);
+  }
+
+  void HandleTranslationUnit(clang::ASTContext &Context) override {
+TraverseDecl(Context.getTranslationUnitDecl());
+  }
+
+  bool TraverseFunctionDecl(clang::FunctionDecl *Decl) {
+Context->getToolResults()->addResult(Decl->getNameAsString(), "1");
+return ASTVisitor::TraverseFunctionDecl(Decl);
+  }
+
+private:
+  ExecutionContext *const Context;
+};
+
+class ReportResultAction : public ASTFrontendAction {
+public:
+  explicit ReportResultAction(ExecutionContext *Context) : Context(Context) {
+assert(Context != nullptr);
+  }
+
+protected:
+  std::unique_ptr
+  CreateASTConsumer(clang::CompilerInstance &compiler,
+StringRef /* dummy */) override {
+std::unique_ptr ast_consumer{
+new ASTConsumerWithResult(Context)};
+return ast_consumer;
+  }
+
+private:
+  ExecutionContext *const Context;
+};
+
+class ReportResultActionFactory : public FrontendActionFactory {
+public:
+  ReportResultActionFactory(ExecutionContext *Context) : Context(Context) {}
+  FrontendAction *create() override { return new ReportResultAction(Context); }
+
+private:
+  ExecutionContext *const Context;
+};
+
+} // namespace
+
+class TestToolExecutor : public ToolExecutor {
+public:
+  static const char *ExecutorName;
+
+  TestToolExecutor(CommonOptionsParser Options)
+  : OptionsParser(std::move(Options)) {}
+
+  StringRef getExecutorName() const override { return ExecutorName; }
+
+  llvm::Error
+  execute(llvm::ArrayRef<
+  std::pair>>)
+  override {
+return llvm::Error::success();
+  }
+
+  ExecutionContext *getExecutionContext() override { return nullptr; };
+
+  llvm::ArrayRef getSourcePaths() const {
+return OptionsParser.getSourcePathList();
+  }
+
+  void mapVirtualFile(StringRef FilePath, StringRef Content) override {
+VFS[FilePath] = Content;
+  }
+
+private:
+  CommonOptionsParser OptionsParser;
+  std::string SourcePaths;
+  std::map VFS;
+};
+
+const char *TestToolExecutor::ExecutorName = "test-executor";
+
+class TestToolExecutorPlugin : public ToolExecutorPlugin {
+public:
+  llvm::Expected>
+  create(CommonOptionsParser &OptionsParser) override {
+return llvm::make_unique(std::move(OptionsParser));
+  }
+};
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the plugin.
+extern volatile int ToolExecutorPluginAnchorSource;
+
+static int LLVM_ATTRIBUTE_UNUSED TestToolExecutorPluginAnchorDest =
+ToolExecutorPluginAnchorSource;
+
+static ToolExecutorPluginRegistry::Add
+X("test-executor", "Plugin for TestToolExecutor.");
+
+llvm::cl::OptionCategory TestCategory("execution-test options");
+
+TEST(CreateToolExecutorTest, F

[PATCH] D39127: Fix template parameter default args missed if redecled

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D39127#904685, @rnk wrote:

> I think you forgot to svn add the test case


Ugg... thanks for the heads up!  Added in r316437


Repository:
  rL LLVM

https://reviews.llvm.org/D39127



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

[PATCH] D39241: [clang-rename] Fix and enable the failing TemplatedClassFunction test.

[Haojian Wu via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D39241

Files:
  lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  test/clang-rename/TemplatedClassFunction.cpp


Index: test/clang-rename/TemplatedClassFunction.cpp
===
--- test/clang-rename/TemplatedClassFunction.cpp
+++ test/clang-rename/TemplatedClassFunction.cpp
@@ -6,17 +6,14 @@
 
 int main(int argc, char **argv) {
   A a;
-  a.foo();   /* Test 2 */ // CHECK: a.bar()   /* Test 2 */
+  a.foo();   /* Test 2 */ // CHECK: a.bar();   /* Test 2 */
   return 0;
 }
 
 // Test 1.
-// RUN: clang-refactor rename -offset=48 -new-name=bar %s -- | sed 's,//.*,,' 
| FileCheck %s
+// RUN: clang-rename -offset=48 -new-name=bar %s -- | sed 's,//.*,,' | 
FileCheck %s
 // Test 2.
-// RUN: clang-refactor rename -offset=162 -new-name=bar %s -- | sed 's,//.*,,' 
| FileCheck %s
-//
-// Currently unsupported test.
-// XFAIL: *
+// RUN: clang-rename -offset=162 -new-name=bar %s -- | sed 's,//.*,,' | 
FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'foo.*' 
Index: lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -73,6 +73,13 @@
 if (checkIfOverriddenFunctionAscends(OverriddenMethod))
   USRSet.insert(getUSRForDecl(OverriddenMethod));
   }
+
+  // If MethodDecl is an instantiated member function of a class template
+  // specialiazation, we also need to add the USR of the template member
+  // function from which MethodDecl is instantiated, otherwise the template
+  // member function would not be renamed.
+  if (const auto *FT = MethodDecl->getInstantiatedFromMemberFunction())
+USRSet.insert(getUSRForDecl(FT));
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
@@ -84,9 +91,16 @@
 return std::vector(USRSet.begin(), USRSet.end());
   }
 
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
   bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
 if (MethodDecl->isVirtual())
   OverriddenMethods.push_back(MethodDecl);
+// If FoundDecl is a template member function, all corresponding
+// instantiated member functions should be included, otherwise only the
+// template member function would be renamed.
+if (MethodDecl->getInstantiatedFromMemberFunction() == FoundDecl)
+  USRSet.insert(getUSRForDecl(MethodDecl));
 return true;
   }
 


Index: test/clang-rename/TemplatedClassFunction.cpp
===
--- test/clang-rename/TemplatedClassFunction.cpp
+++ test/clang-rename/TemplatedClassFunction.cpp
@@ -6,17 +6,14 @@
 
 int main(int argc, char **argv) {
   A a;
-  a.foo();   /* Test 2 */ // CHECK: a.bar()   /* Test 2 */
+  a.foo();   /* Test 2 */ // CHECK: a.bar();   /* Test 2 */
   return 0;
 }
 
 // Test 1.
-// RUN: clang-refactor rename -offset=48 -new-name=bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=48 -new-name=bar %s -- | sed 's,//.*,,' | FileCheck %s
 // Test 2.
-// RUN: clang-refactor rename -offset=162 -new-name=bar %s -- | sed 's,//.*,,' | FileCheck %s
-//
-// Currently unsupported test.
-// XFAIL: *
+// RUN: clang-rename -offset=162 -new-name=bar %s -- | sed 's,//.*,,' | FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'foo.*' 
Index: lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -73,6 +73,13 @@
 if (checkIfOverriddenFunctionAscends(OverriddenMethod))
   USRSet.insert(getUSRForDecl(OverriddenMethod));
   }
+
+  // If MethodDecl is an instantiated member function of a class template
+  // specialiazation, we also need to add the USR of the template member
+  // function from which MethodDecl is instantiated, otherwise the template
+  // member function would not be renamed.
+  if (const auto *FT = MethodDecl->getInstantiatedFromMemberFunction())
+USRSet.insert(getUSRForDecl(FT));
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
@@ -84,9 +91,16 @@
 return std::vector(USRSet.begin(), USRSet.end());
   }
 
+  bool shouldVisitTemplateInstantiations() const { return true; }
+
   bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
 if (MethodDecl->isVirtual())
   OverriddenMethods.push_back(MethodDecl);
+// If FoundDecl is a template member function, all corresponding
+// insta

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

[Eric Liu via Phabricator via cfe-commits]

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

PTAL

I experimented with our internal tools (incl. mapreduce-based execution) and 
modified the interfaces a bit:
o Get rid of `ExecutionConfig` class; pass actions into executors directly.
o Moved `ArgumentAdjuster`s that are common to all actions into 
`ExecutionContext`. 
o Make command-line based executor selection more explicit by requiring an 
`--executor` option.


https://reviews.llvm.org/D34272



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

[libcxx] r316439 - Mark string_view literals as 'noexcept'. Fixes PR#25054. Thanks to Pavel K for the bug report.

[Marshall Clow via cfe-commits]

Author: marshall
Date: Tue Oct 24 07:06:00 2017
New Revision: 316439

URL: http://llvm.org/viewvc/llvm-project?rev=316439&view=rev
Log:
Mark string_view literals as 'noexcept'. Fixes PR#25054. Thanks to Pavel K for 
the bug report.

Modified:
libcxx/trunk/include/string_view

libcxx/trunk/test/std/strings/string.view/string_view.literals/literal.pass.cpp

Modified: libcxx/trunk/include/string_view
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string_view?rev=316439&r1=316438&r2=316439&view=diff
==
--- libcxx/trunk/include/string_view (original)
+++ libcxx/trunk/include/string_view Tue Oct 24 07:06:00 2017
@@ -155,10 +155,10 @@ namespace std {
   template <> struct hash;
   template <> struct hash;
 
-  constexpr basic_string operator "" s( const char *str, size_t 
len ); // C++17
-  constexpr basic_string  operator "" s( const wchar_t *str,  size_t 
len ); // C++17
-  constexpr basic_string operator "" s( const char16_t *str, size_t 
len ); // C++17
-  constexpr basic_string operator "" s( const char32_t *str, size_t 
len ); // C++17
+  constexpr basic_string_view operator "" sv( const char *str, 
size_t len ) noexcept;
+  constexpr basic_string_view  operator "" sv( const wchar_t *str,  
size_t len ) noexcept;
+  constexpr basic_string_view operator "" sv( const char16_t *str, 
size_t len ) noexcept;
+  constexpr basic_string_view operator "" sv( const char32_t *str, 
size_t len ) noexcept;
 
 }  // namespace std
 
@@ -760,25 +760,25 @@ inline namespace literals
   inline namespace string_view_literals
   {
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-basic_string_view operator "" sv(const char *__str, size_t __len)
+basic_string_view operator "" sv(const char *__str, size_t __len) 
_NOEXCEPT
 {
 return basic_string_view (__str, __len);
 }
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-basic_string_view operator "" sv(const wchar_t *__str, size_t 
__len)
+basic_string_view operator "" sv(const wchar_t *__str, size_t 
__len) _NOEXCEPT
 {
 return basic_string_view (__str, __len);
 }
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-basic_string_view operator "" sv(const char16_t *__str, size_t 
__len)
+basic_string_view operator "" sv(const char16_t *__str, size_t 
__len) _NOEXCEPT
 {
 return basic_string_view (__str, __len);
 }
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-basic_string_view operator "" sv(const char32_t *__str, size_t 
__len)
+basic_string_view operator "" sv(const char32_t *__str, size_t 
__len) _NOEXCEPT
 {
 return basic_string_view (__str, __len);
 }

Modified: 
libcxx/trunk/test/std/strings/string.view/string_view.literals/literal.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string_view.literals/literal.pass.cpp?rev=316439&r1=316438&r2=316439&view=diff
==
--- 
libcxx/trunk/test/std/strings/string.view/string_view.literals/literal.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/strings/string.view/string_view.literals/literal.pass.cpp 
Tue Oct 24 07:06:00 2017
@@ -54,4 +54,10 @@ int main()
 static_assert( L"ABC"sv.size() == 3, "");
 static_assert( u"ABC"sv.size() == 3, "");
 static_assert( U"ABC"sv.size() == 3, "");
+
+static_assert(noexcept(  "ABC"sv), "");
+static_assert(noexcept(u8"ABC"sv), "");
+static_assert(noexcept( L"ABC"sv), "");
+static_assert(noexcept( u"ABC"sv), "");
+static_assert(noexcept( U"ABC"sv), "");
 }


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

[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D39138#904747, @rjmccall wrote:

> Okay, if this is just for your own checking, I'd rather not take it.  It's 
> not a significant compile-time cost, but there's no reason to pay it at all.


In that case, can we take it? I'd rather have everything decorated for use by 
the type sanitizer.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



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

[PATCH] D39241: [clang-rename] Fix and enable the failing TemplatedClassFunction test.

[Eric Liu via Phabricator via cfe-commits]

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

lg




Comment at: test/clang-rename/TemplatedClassFunction.cpp:8
 int main(int argc, char **argv) {
   A a;
+  a.foo();   /* Test 2 */ // CHECK: a.bar();   /* Test 2 */

Could you add a few more instantiations in the test case, e.g. `A`, 
`A`?


https://reviews.llvm.org/D39241



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

r316444 - [modules] Add a regression test for merging anon decls in extern C contexts.

[Vassil Vassilev via cfe-commits]

Author: vvassilev
Date: Tue Oct 24 07:52:35 2017
New Revision: 316444

URL: http://llvm.org/viewvc/llvm-project?rev=316444&view=rev
Log:
[modules] Add a regression test for merging anon decls in extern C contexts.

Added:
cfe/trunk/test/Modules/merge-anon-in-extern_c.cpp

Added: cfe/trunk/test/Modules/merge-anon-in-extern_c.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-anon-in-extern_c.cpp?rev=316444&view=auto
==
--- cfe/trunk/test/Modules/merge-anon-in-extern_c.cpp (added)
+++ cfe/trunk/test/Modules/merge-anon-in-extern_c.cpp Tue Oct 24 07:52:35 2017
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -verify %s
+// expected-no-diagnostics
+
+#pragma clang module build sys_types
+module sys_types {}
+#pragma clang module contents
+#pragma clang module begin sys_types
+extern "C" {
+  typedef union { bool b; } pthread_mutex_t;
+}
+#pragma clang module end
+#pragma clang module endbuild
+
+typedef union { bool b; } pthread_mutex_t;
+#pragma clang module import sys_types
+
+const pthread_mutex_t *m;
+


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

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

[Yaxun Liu via Phabricator via cfe-commits]

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

Revised the test by Paul's comments.


https://reviews.llvm.org/D39069

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenOpenCL/func-call-dbg-loc.cl


Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 
-emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg ![[LOC:[0-9]+]]
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg ![[LOC]]
+func2(func1());
+}
+
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -74,12 +74,11 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);


Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 -emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg ![[LOC:[0-9]+]]
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg ![[LOC]]
+func2(func1());
+}
+
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -74,12 +74,11 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) {
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39064: implement __has_unique_object_representations

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/Type.cpp:2226
+Context.getFieldOffset(*Record->field_begin()));
+for (const auto *Field : Record->fields()) {
+  if (!Field->getType().hasUniqueObjectRepresentations(Context))

erichkeane wrote:
> rnk wrote:
> > What about base classes? I think that's where the padding detection is 
> > going to get wacky. =/
> Based on my reading of the RecordLayout stuff, the "getFieldOffset" should 
> take that into account, right?  It seems that 'fields' contains all fields, 
> and thus should run through the offset of all of them, right?  I'll add 
> another test that inherits from a Padded struct to verify (as well as one 
> that causes padding with the inheritence).
@rnk: You're correct here I believe, I'm not properly checking the base class 
as I thought I was.  I'll get a new patch later today.


https://reviews.llvm.org/D39064



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

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

[Eric Liu via Phabricator via cfe-commits]

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

Lgtm




Comment at: include/clang/Basic/DiagnosticRefactoringKinds.td:24
+  "not overlap with the AST nodes of interest">;
+
+def err_refactor_code_outside_of_function : Error<"the selected code is not a "

nit: was this newline intended?



Comment at: tools/clang-refactor/TestSupport.cpp:370
+   EndColumn);
+} else
+  EndOffset = Offset;

nit: curly braces in else-branch if if-branch has braces.


Repository:
  rL LLVM

https://reviews.llvm.org/D38982



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

r316447 - Fix spelling in comment, field is isMsStruct, not Strust

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Oct 24 08:34:59 2017
New Revision: 316447

URL: http://llvm.org/viewvc/llvm-project?rev=316447&view=rev
Log:
Fix spelling in comment, field is isMsStruct, not Strust

Modified:
cfe/trunk/include/clang/AST/Decl.h

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=316447&r1=316446&r2=316447&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue Oct 24 08:34:59 2017
@@ -3540,7 +3540,7 @@ public:
 return K >= firstRecord && K <= lastRecord;
   }
 
-  /// isMsStrust - Get whether or not this is an ms_struct which can
+  /// isMsStruct - Get whether or not this is an ms_struct which can
   /// be turned on with an attribute, pragma, or -mms-bitfields
   /// commandline option.
   bool isMsStruct(const ASTContext &C) const;


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

[PATCH] D39055: [refactor] Add an editor client that is used in clangd

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: include/clang/Tooling/Refactoring/EditorClient.h:46
+  /// error otherwise.
+  Expected performRefactoring(ASTContext &Context,
+ StringRef CommandName,

I think it's worth thinking about how this would work for refactoring actions 
that touch multiple TUs/ASTs, e.g. renaming symbol references in all user files 
or adding function definition (in cpp file) from a function declaration (in 
header file). With the current interfaces, clang-refactor would only be able to 
work on a single AST, but I think it should be able to get more ASTs from 
clangd if needed.

The clangd folks (including me) are happy to provide an interface that serves 
ASTs; we just need to make sure clang-refactor handles multi-TU refactoring. 
WDYT?


Repository:
  rL LLVM

https://reviews.llvm.org/D39055



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D37813#876227, @klimek wrote:

> I think instead of introducing more and more special cases of macros we might 
> want to handle, we should instead allow specifying macro productions globally.


what do you mean?
Do you mean to group all the macro configuration options into "Macros" field, 
still containing one field for each kind of macro? Or do you have something 
else in mind?


https://reviews.llvm.org/D37813



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

Re: r316447 - Fix spelling in comment, field is isMsStruct, not Strust

[Nico Weber via cfe-commits]

This is fine, but a few years ago we agreed that we shouldn't repeat method
names in doxygen. So an Even Better Fix would've been to just remove
everything up to (and including) the ' - ' :-) Just FYI, no need to follow
this up with the deletion.

On Tue, Oct 24, 2017 at 3:34 PM, Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Tue Oct 24 08:34:59 2017
> New Revision: 316447
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316447&view=rev
> Log:
> Fix spelling in comment, field is isMsStruct, not Strust
>
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/Decl.h?rev=316447&r1=316446&r2=316447&view=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Tue Oct 24 08:34:59 2017
> @@ -3540,7 +3540,7 @@ public:
>  return K >= firstRecord && K <= lastRecord;
>}
>
> -  /// isMsStrust - Get whether or not this is an ms_struct which can
> +  /// isMsStruct - Get whether or not this is an ms_struct which can
>/// be turned on with an attribute, pragma, or -mms-bitfields
>/// commandline option.
>bool isMsStruct(const ASTContext &C) const;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r316447 - Fix spelling in comment, field is isMsStruct, not Strust

[Keane, Erich via cfe-commits]

Ah, thanks for letting me know! I’ll do that now.

From: tha...@google.com [mailto:tha...@google.com] On Behalf Of Nico Weber
Sent: Tuesday, October 24, 2017 8:59 AM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r316447 - Fix spelling in comment, field is isMsStruct, not Strust

This is fine, but a few years ago we agreed that we shouldn't repeat method 
names in doxygen. So an Even Better Fix would've been to just remove everything 
up to (and including) the ' - ' :-) Just FYI, no need to follow this up with 
the deletion.

On Tue, Oct 24, 2017 at 3:34 PM, Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Tue Oct 24 08:34:59 2017
New Revision: 316447

URL: http://llvm.org/viewvc/llvm-project?rev=316447&view=rev
Log:
Fix spelling in comment, field is isMsStruct, not Strust

Modified:
cfe/trunk/include/clang/AST/Decl.h

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=316447&r1=316446&r2=316447&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue Oct 24 08:34:59 2017
@@ -3540,7 +3540,7 @@ public:
 return K >= firstRecord && K <= lastRecord;
   }

-  /// isMsStrust - Get whether or not this is an ms_struct which can
+  /// isMsStruct - Get whether or not this is an ms_struct which can
   /// be turned on with an attribute, pragma, or -mms-bitfields
   /// commandline option.
   bool isMsStruct(const ASTContext &C) const;


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

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

r316453 - Remove repeated function name in doxygen comment.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Tue Oct 24 09:16:34 2017
New Revision: 316453

URL: http://llvm.org/viewvc/llvm-project?rev=316453&view=rev
Log:
Remove repeated function name in doxygen comment.

Modified:
cfe/trunk/include/clang/AST/Decl.h

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=316453&r1=316452&r2=316453&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue Oct 24 09:16:34 2017
@@ -3540,7 +3540,7 @@ public:
 return K >= firstRecord && K <= lastRecord;
   }
 
-  /// isMsStruct - Get whether or not this is an ms_struct which can
+  /// \brief Get whether or not this is an ms_struct which can
   /// be turned on with an attribute, pragma, or -mms-bitfields
   /// commandline option.
   bool isMsStruct(const ASTContext &C) const;


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

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


https://reviews.llvm.org/D32478



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

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> My question is: if CanBreak is false, we currently don't call 
> breakProtrudingToken. So either we do something very wrong in that case 
> (which might be true, but I'd like to understand why) or we should be able  
> to just calculate the penalty by not breaking anything and go on.

CanBreak is currently very short, it only verifies some very broad conditions. 
I initiallly tried patching at this level, but it really does not work.

Most conditions are actually tested at the beginning of breakProtrudingToken. 
That part of the code actually takes different branches for different kind of 
tokens (strings, block commands, line comments...), and handles many different 
cases for each. This goal is to create the actual BreakableToken object, but it 
has a few other side effects: it returns immediately in various corner cases 
(javascript, preprocessor, formatting macros), and it tweaks some 
variables, e.g. ColumnLimit.

In addition, replacement is non trivial even in case we choose not to reflow: 
it is required in order to properly manage whitespaces. For this reason we 
really must pass through the loop in most cases.


https://reviews.llvm.org/D33589



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

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

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 120084.
Typz added a comment.

rebase


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2420,6 +2420,42 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("void f() {\n"
+   "  FOO(a)\n"
+   "  return a;\n"
+   "}",
+   Style);
+  verifyFormat("FOO(a)\n"
+   "FOO(b)",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "FOO(b)\n"
+   "int c = 0;",
+   Style);
+  verifyFormat("int a = 0;\n"
+   "int x = FOO(a)\n"
+   "int b = 0;",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10244,6 +10280,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeCategories.clear();
   std::vector ExpectedCategories = {{"abc/.*", 2},
   {".*", 1}};
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1093,6 +1093,15 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
@@ -1242,6 +1251,16 @@
 return;
   }
 
+  if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+nextToken();
+if (FormatTok->is(tok::l_paren))
+  parseParens();
+if (FormatTok->is(tok::semi))
+  nextToken();
+addUnwrappedLine();
+return;
+  }
+
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "llvm/Support/Regex.h"
+#include 
 
 #include 
 
@@ -97,7 +98,8 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  llvm::SmallMapVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -37,8 +37,9 @@
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string &ForEachMacro : Style.ForEachMacros)
-ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
-  std::sort(ForEachMacros.begin(), ForEachMacros.end());
+Macros.insert({&IdentTable.get(ForEachMacro), TT_ForEachMacro});
+  for (const std::string &StatementMacro : Style.StatementMacros)
+Macros.insert({&IdentTable.get(StatementMacro), TT_StatementMacro});
 }
 
 ArrayRef FormatTokenLexer::lex() {
@@ -626,12 +627,13 @@
   }
 
   if (Style.isCpp()) {
+decltype(Macros)::iterator it;
 if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
-std::find(ForEachMacros.begin(), ForEachMacros.end(),
-  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
- 

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

In https://reviews.llvm.org/D39204#904312, @efriedma wrote:

> The gcc documentation says "GCC includes built-in versions of many of the 
> functions in the standard C library. These functions come in two forms: one 
> whose names start with the `__builtin_` prefix, and the other without. Both 
> forms have the same type (including prototype), the same address (when their 
> address is taken), and the same meaning as the C library functions".  And gcc 
> specifically preserves the stated semantics.  Given that, I'm not sure it 
> makes sense for us to try to redefine `__builtin_sqrt()` just because it's 
> convenient.
>
> Note that this reasoning only applies if the user hasn't specified any 
> fast-math flags; under -ffinite-math-only, we can assume the result isn't a 
> NaN, and therefore we can use `llvm.sqrt.*`. (The definition of `llvm.sqrt.*` 
> changed in https://reviews.llvm.org/D28797; I don't think we ever updated 
> clang to take advantage of this).
>
> If we really need a name for the never-sets-errno sqrt, we should probably 
> use a different name, e.g. `__builtin_ieee_sqrt()`.


Thanks for the explanation and link. Let me know if I've gone wrong:

1. We don't want to convert clang math builtins to llvm intrinsics because 
builtins are supposed to be exactly equivalent to C library functions 
(including setting errno).
2. LLVM intrinsics should be equivalent to C library functions except that they 
don't set errno (but this is currently wrong in some cases, and 
https://reviews.llvm.org/D28335 would fix that).
3. Therefore, the existing code in this file that is converting 'pow' and other 
builtin calls to intrinsics is correct for now, but only because 2 wrongs made 
it right? :)


https://reviews.llvm.org/D39204



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

[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 120085.
Typz added a comment.

rebase


https://reviews.llvm.org/D37813

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"} /* end of TESTSUITE(A) */",
+

[libcxx] r316456 - Mark string_view's constructor from (ptr, len) as noexcept (an extension). Update the tests to check this (and other noexcept bits

[Marshall Clow via cfe-commits]

Author: marshall
Date: Tue Oct 24 09:30:06 2017
New Revision: 316456

URL: http://llvm.org/viewvc/llvm-project?rev=316456&view=rev
Log:
Mark string_view's constructor from (ptr,len) as noexcept (an extension). 
Update the tests to check this (and other noexcept bits

Modified:
libcxx/trunk/include/string_view
libcxx/trunk/test/std/strings/string.view/string.view.cons/default.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.cons/from_literal.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.cons/from_ptr_len.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.cons/from_string.pass.cpp

Modified: libcxx/trunk/include/string_view
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string_view?rev=316456&r1=316455&r2=316456&view=diff
==
--- libcxx/trunk/include/string_view (original)
+++ libcxx/trunk/include/string_view Tue Oct 24 09:30:06 2017
@@ -216,7 +216,7 @@ public:
basic_string_view& operator=(const basic_string_view&) _NOEXCEPT = 
default;
 
_LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
-   basic_string_view(const _CharT* __s, size_type __len)
+   basic_string_view(const _CharT* __s, size_type __len) _NOEXCEPT
: __data(__s), __size(__len)
{
 // #if _LIBCPP_STD_VER > 11

Modified: 
libcxx/trunk/test/std/strings/string.view/string.view.cons/default.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string.view.cons/default.pass.cpp?rev=316456&r1=316455&r2=316456&view=diff
==
--- libcxx/trunk/test/std/strings/string.view/string.view.cons/default.pass.cpp 
(original)
+++ libcxx/trunk/test/std/strings/string.view/string.view.cons/default.pass.cpp 
Tue Oct 24 09:30:06 2017
@@ -21,6 +21,8 @@ template
 void test () {
 #if TEST_STD_VER > 11
 {
+ASSERT_NOEXCEPT(T());
+
 constexpr T sv1;
 static_assert ( sv1.size() == 0, "" );
 static_assert ( sv1.empty(), "");

Modified: 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_literal.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string.view.cons/from_literal.pass.cpp?rev=316456&r1=316455&r2=316456&view=diff
==
--- 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_literal.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_literal.pass.cpp
 Tue Oct 24 09:30:06 2017
@@ -30,7 +30,11 @@ size_t StrLen ( const CharT *s ) {
 
 template
 void test ( const CharT *s ) {
-std::basic_string_view sv1 ( s );
+typedef std::basic_string_view SV;
+//  I'd love to do this, but it would require traits::length() to be noexcept
+//  LIBCPP_ASSERT_NOEXCEPT(SV(s));
+
+SV sv1 ( s );
 assert ( sv1.size() == StrLen( s ));
 assert ( sv1.data() == s );
 }

Modified: 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_ptr_len.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string.view.cons/from_ptr_len.pass.cpp?rev=316456&r1=316455&r2=316456&view=diff
==
--- 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_ptr_len.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_ptr_len.pass.cpp
 Tue Oct 24 09:30:06 2017
@@ -23,7 +23,10 @@
 template
 void test ( const CharT *s, size_t sz ) {
 {
-std::basic_string_view sv1 ( s, sz );
+typedef std::basic_string_view SV;
+LIBCPP_ASSERT_NOEXCEPT(SV(s, sz));
+
+SV sv1 ( s, sz );
 assert ( sv1.size() == sz );
 assert ( sv1.data() == s );
 }

Modified: 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_string.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/string.view.cons/from_string.pass.cpp?rev=316456&r1=316455&r2=316456&view=diff
==
--- 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_string.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/strings/string.view/string.view.cons/from_string.pass.cpp 
Tue Oct 24 09:30:06 2017
@@ -24,7 +24,10 @@ struct dummy_char_traits : public std::c
 
 template
 void test ( const std::basic_string &str ) {
-std::basic_string_view sv1 ( str );
+typedef std::basic_string_view SV;
+ASSERT_NOEXCEPT(SV(str));
+
+SV sv1 ( str );
 assert ( sv1.size() == str.size());
 assert ( sv1.data() == str.data());
 }


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

r316458 - [code completion] Complete ObjC methods in @implementation without leading

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Oct 24 09:39:37 2017
New Revision: 316458

URL: http://llvm.org/viewvc/llvm-project?rev=316458&view=rev
Log:
[code completion] Complete ObjC methods in @implementation without leading
'-'/'+' prefix

rdar://12040840

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/Index/complete-method-decls.m

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=316458&r1=316457&r2=316458&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Oct 24 09:39:37 2017
@@ -10210,8 +10210,7 @@ public:
   void CodeCompleteObjCPropertyDefinition(Scope *S);
   void CodeCompleteObjCPropertySynthesizeIvar(Scope *S,
   IdentifierInfo *PropertyName);
-  void CodeCompleteObjCMethodDecl(Scope *S,
-  bool IsInstanceMethod,
+  void CodeCompleteObjCMethodDecl(Scope *S, Optional IsInstanceMethod,
   ParsedType ReturnType);
   void CodeCompleteObjCMethodDeclSelector(Scope *S,
   bool IsInstanceMethod,

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=316458&r1=316457&r2=316458&view=diff
==
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Tue Oct 24 09:39:37 2017
@@ -753,9 +753,15 @@ Parser::ParseExternalDeclaration(ParsedA
 SingleDecl = ParseObjCMethodDefinition();
 break;
   case tok::code_completion:
-  Actions.CodeCompleteOrdinaryName(getCurScope(), 
- CurParsedObjCImpl? Sema::PCC_ObjCImplementation
-  : Sema::PCC_Namespace);
+if (CurParsedObjCImpl) {
+  // Code-complete Objective-C methods even without leading '-'/'+' prefix.
+  Actions.CodeCompleteObjCMethodDecl(getCurScope(),
+ /*IsInstanceMethod=*/None,
+ /*ReturnType=*/nullptr);
+}
+Actions.CodeCompleteOrdinaryName(
+getCurScope(),
+CurParsedObjCImpl ? Sema::PCC_ObjCImplementation : 
Sema::PCC_Namespace);
 cutOffParsing();
 return nullptr;
   case tok::kw_export:

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=316458&r1=316457&r2=316458&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Tue Oct 24 09:39:37 2017
@@ -6647,7 +6647,7 @@ typedef llvm::DenseMap<
 /// indexed by selector so they can be easily found.
 static void FindImplementableMethods(ASTContext &Context,
  ObjCContainerDecl *Container,
- bool WantInstanceMethods,
+ Optional WantInstanceMethods,
  QualType ReturnType,
  KnownMethodsMap &KnownMethods,
  bool InOriginalClass = true) {
@@ -6718,7 +6718,7 @@ static void FindImplementableMethods(AST
   // we want the methods from this container to override any methods
   // we've previously seen with the same selector.
   for (auto *M : Container->methods()) {
-if (M->isInstanceMethod() == WantInstanceMethods) {
+if (!WantInstanceMethods || M->isInstanceMethod() == *WantInstanceMethods) 
{
   if (!ReturnType.isNull() &&
   !Context.hasSameUnqualifiedType(ReturnType, M->getReturnType()))
 continue;
@@ -7390,8 +7390,7 @@ static void AddObjCKeyValueCompletions(O
   }
 }
 
-void Sema::CodeCompleteObjCMethodDecl(Scope *S, 
-  bool IsInstanceMethod,
+void Sema::CodeCompleteObjCMethodDecl(Scope *S, Optional 
IsInstanceMethod,
   ParsedType ReturnTy) {
   // Determine the return type of the method we're declaring, if
   // provided.
@@ -7446,7 +7445,13 @@ void Sema::CodeCompleteObjCMethodDecl(Sc
 ObjCMethodDecl *Method = M->second.getPointer();
 CodeCompletionBuilder Builder(Results.getAllocator(),
   Results.getCodeCompletionTUInfo());
-
+
+// Add the '-'/'+' prefix if it wasn't provided yet.
+if (!IsInstanceMethod) {
+  Builder.AddTextChunk(Method->isInstanceMethod() ? "-" : "+");
+  Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+}
+
 // If the result type was not already provided, add it to the
 // pattern as (type).
  

[PATCH] D36790: [ObjC] Messages to 'self' in class methods should use class method dispatch to avoid multiple method ambiguities

[Doug Gregor via Phabricator via cfe-commits]

doug.gregor added a comment.

Yes, I think it's reasonable to treat instancetype as an inherited requirement. 
I guess the only exception would be if we had some notion of final classes or 
methods in Objective-C, in which case they'd be able to return a concrete type.


Repository:
  rL LLVM

https://reviews.llvm.org/D36790



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added subscribers: vit9696, hfinkel.
hfinkel added a comment.

Noting that, as @vit9696  pointed out in https://reviews.llvm.org/D38554, this 
does not suppress uses of the PLT that occur from backend/optimizer-generated 
functions (e.g., calls into compiler-rt and similar).


https://reviews.llvm.org/D39079



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

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

Working my way through the stack: does the sqrt LangRef change mean we can 
revert https://reviews.llvm.org/rL265521? What allows us to transform any of 
those libm calls that set errno to vectors in the first place?

I'm even more confused than usual, but if I can find a way to untangle this 
mess, I'll try to start making patches.


https://reviews.llvm.org/D39204



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

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: lib/Driver/ToolChain.cpp:318
+  else
+OSLibName = getOS();
   llvm::sys::path::append(Path, "lib", OSLibName);

dschuff wrote:
> Is this logic intended to replace what was removed from CommonArgs.cpp? 
> Should there be an assert here too?
Just didn't see the point of that assert.  Can you see what the intention might 
be?  I don't see why AddRunTimeLibs() should be callable for any/all triples, 
do you?  I would have had to add  llvm::Triple::Unknown to the list of 
supported OSs, which seemed strange.


https://reviews.llvm.org/D39218



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Why again is this a good idea? This is an even worse hack than -Bsymbolic, the 
latter at least is visible in ELF header without code inspection. This is 
breaking core premises of ELF.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In https://reviews.llvm.org/D39079#905353, @hfinkel wrote:

> Noting that, as @vit9696  pointed out in https://reviews.llvm.org/D38554, 
> this does not suppress uses of the PLT that occur from 
> backend/optimizer-generated functions (e.g., calls into compiler-rt and 
> similar).


Can I work on this as a follow-up?


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In https://reviews.llvm.org/D39079#905372, @joerg wrote:

> Why again is this a good idea? This is an even worse hack than -Bsymbolic, 
> the latter at least is visible in ELF header without code inspection. This is 
> breaking core premises of ELF.


Could you elaborate a bit more on what ELF promises this is breaking?  I 
haven't fully read through https://reviews.llvm.org/D38554 yet.


https://reviews.llvm.org/D39079



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

r316465 - [refactor] Initial outline of implementation of "extract function" refactoring

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Oct 24 10:18:45 2017
New Revision: 316465

URL: http://llvm.org/viewvc/llvm-project?rev=316465&view=rev
Log:
[refactor] Initial outline of implementation of "extract function" refactoring

This commit adds an initial, skeleton outline of the "extract function"
refactoring. The extracted function doesn't capture variables / rewrite code
yet, it just basically does a simple copy-paste.
The following initiation rules are specified:

- extraction can only be done for executable code in a function/method/block.
  This means that you can't extract a global variable initialize into a function
  right now.
- simple literals and references are not extractable.

This commit also adds support for full source ranges to clang-refactor's test
mode.

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

Added:
cfe/trunk/lib/Tooling/Refactoring/ASTSelectionRequirements.cpp
cfe/trunk/lib/Tooling/Refactoring/Extract.cpp
cfe/trunk/test/Refactor/Extract/
cfe/trunk/test/Refactor/Extract/ExtractExprIntoFunction.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td
cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def

cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringRuleContext.h
cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
cfe/trunk/test/Refactor/LocalRename/Field.cpp
cfe/trunk/test/Refactor/LocalRename/NoSymbolSelectedError.cpp
cfe/trunk/test/Refactor/tool-test-support.c
cfe/trunk/tools/clang-refactor/TestSupport.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td?rev=316465&r1=316464&r2=316465&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td Tue Oct 24 
10:18:45 2017
@@ -19,6 +19,13 @@ def err_refactor_no_selection : Error<"r
   "without a selection">;
 def err_refactor_selection_no_symbol : Error<"there is no symbol at the given "
   "location">;
+def err_refactor_selection_invalid_ast : Error<"the provided selection does "
+  "not overlap with the AST nodes of interest">;
+
+def err_refactor_code_outside_of_function : Error<"the selected code is not a "
+  "part of a function's / method's body">;
+def err_refactor_extract_simple_expression : Error<"the selected expression "
+  "is too simple to extract">;
 
 }
 

Modified: cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h?rev=316465&r1=316464&r2=316465&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h Tue Oct 24 
10:18:45 2017
@@ -115,6 +115,21 @@ public:
 return SelectedNode.get().Children[I].Node.get();
   }
 
+  /// Returns true when a selected code range is in a function-like body
+  /// of code, like a function, method or a block.
+  ///
+  /// This function can be used to test against selected expressions that are
+  /// located outside of a function, e.g. global variable initializers, default
+  /// argument values, or even template arguments.
+  ///
+  /// Use the \c getFunctionLikeNearestParent to get the function-like parent
+  /// declaration.
+  bool isInFunctionLikeBodyOfCode() const;
+
+  /// Returns the nearest function-like parent declaration or null if such
+  /// declaration doesn't exist.
+  const Decl *getFunctionLikeNearestParent() const;
+
   static Optional
   create(SourceRange SelectionRange, const SelectedASTNode &ASTSelection);
 

Modified: 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def?rev=316465&r1=316464&r2=316465&view=diff
==
--- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def 
(original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def 
Tue Oct 24 10:18:45 2017
@@ -3,5 +3,6 @@
 #endif
 
 REFACTORING_ACTION(LocalRename)
+REFACTORING_ACTION(Extract)
 
 #undef REFACTORING_ACTION

Modified: 
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h?r

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
arphaman marked an inline comment as done.
Closed by commit rL316465: [refactor] Initial outline of implementation of 
"extract function" refactoring (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D38982?vs=119711&id=120095#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38982

Files:
  cfe/trunk/include/clang/Basic/DiagnosticRefactoringKinds.td
  cfe/trunk/include/clang/Tooling/Refactoring/ASTSelection.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRegistry.def
  
cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h
  cfe/trunk/include/clang/Tooling/Refactoring/RefactoringRuleContext.h
  cfe/trunk/lib/Tooling/Refactoring/ASTSelection.cpp
  cfe/trunk/lib/Tooling/Refactoring/ASTSelectionRequirements.cpp
  cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
  cfe/trunk/lib/Tooling/Refactoring/Extract.cpp
  cfe/trunk/test/Refactor/Extract/ExtractExprIntoFunction.cpp
  cfe/trunk/test/Refactor/LocalRename/Field.cpp
  cfe/trunk/test/Refactor/LocalRename/NoSymbolSelectedError.cpp
  cfe/trunk/test/Refactor/tool-test-support.c
  cfe/trunk/tools/clang-refactor/TestSupport.cpp

Index: cfe/trunk/test/Refactor/tool-test-support.c
===
--- cfe/trunk/test/Refactor/tool-test-support.c
+++ cfe/trunk/test/Refactor/tool-test-support.c
@@ -10,10 +10,13 @@
 
 /*range named =+0*/int test5;
 
+/*range =->+0:22*/int test6;
+
 // CHECK: Test selection group '':
 // CHECK-NEXT:   105-105
 // CHECK-NEXT:   158-158
 // CHECK-NEXT:   197-197
+// CHECK-NEXT:   248-251
 // CHECK-NEXT: Test selection group 'named':
 // CHECK-NEXT:   132-132
 // CHECK-NEXT:   218-218
@@ -29,6 +32,8 @@
 // CHECK: invoking action 'local-rename':
 // CHECK-NEXT: -selection={{.*}}tool-test-support.c:9:29
 
+// CHECK: invoking action 'local-rename':
+// CHECK-NEXT: -selection={{.*}}tool-test-support.c:13:19 -> {{.*}}tool-test-support.c:13:22
 
 // The following invocations are in the 'named' group, and they follow
 // the default invocation even if some of their ranges occur prior to the
Index: cfe/trunk/test/Refactor/LocalRename/NoSymbolSelectedError.cpp
===
--- cfe/trunk/test/Refactor/LocalRename/NoSymbolSelectedError.cpp
+++ cfe/trunk/test/Refactor/LocalRename/NoSymbolSelectedError.cpp
@@ -1,5 +1,5 @@
-// RUN: not clang-refactor local-rename -selection=%s:4:1 -new-name=Bar %s -- 2>&1 | FileCheck %s
-// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- 2>&1 | FileCheck --check-prefix=TESTCHECK %s
+// RUN: not clang-refactor local-rename -selection=%s:4:1 -new-name=Bar %s -- 2>&1 | grep -v CHECK | FileCheck %s
+// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- 2>&1 | grep -v CHECK | FileCheck --check-prefix=TESTCHECK %s
 
 class Baz { // CHECK: [[@LINE]]:1: error: there is no symbol at the given location
 };
Index: cfe/trunk/test/Refactor/LocalRename/Field.cpp
===
--- cfe/trunk/test/Refactor/LocalRename/Field.cpp
+++ cfe/trunk/test/Refactor/LocalRename/Field.cpp
@@ -1,9 +1,11 @@
-// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- | grep -v CHECK | FileCheck %s
 
 class Baz {
-  int /*range=*/Foo; // CHECK: int /*range=*/Bar;
+  int /*range=*/Foo;
+  // CHECK: int /*range=*/Bar;
 public:
   Baz();
 };
 
-Baz::Baz() : /*range=*/Foo(0) {}  // CHECK: Baz::Baz() : /*range=*/Bar(0) {};
+Baz::Baz() : /*range=*/Foo(0) {}
+// CHECK: Baz::Baz() : /*range=*/Bar(0) {}
Index: cfe/trunk/test/Refactor/Extract/ExtractExprIntoFunction.cpp
===
--- cfe/trunk/test/Refactor/Extract/ExtractExprIntoFunction.cpp
+++ cfe/trunk/test/Refactor/Extract/ExtractExprIntoFunction.cpp
@@ -0,0 +1,56 @@
+// RUN: clang-refactor extract -selection=test:%s %s -- -std=c++11 2>&1 | grep -v CHECK | FileCheck %s
+
+
+void simpleExtractNoCaptures() {
+  int i = /*range=->+0:33*/1 + 2;
+}
+
+// CHECK: 1 '' results:
+// CHECK:  static int extracted() {
+// CHECK-NEXT: return 1 + 2;{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void simpleExtractNoCaptures() {
+// CHECK-NEXT:   int i = /*range=->+0:33*/extracted();{{$}}
+// CHECK-NEXT: }
+
+void simpleExtractStmtNoCaptures() {
+  /*range astatement=->+1:13*/int a = 1;
+  int b = 2;
+}
+// CHECK: 1 'astatement' results:
+// CHECK:  static void extracted() {
+// CHECK-NEXT: int a = 1;
+// CHECK-NEXT: int b = 2;;{{$}}
+// CHECK-NEXT: }{{[[:space:]].*}}
+// CHECK-NEXT: void simpleExtractStmtNoCaptures() {
+// CHECK-NEXT:   /*range astatement=->+1:13*/extracted(){{$}}
+// CHECK-NEXT: }
+
+
+void blankRangeN

[PATCH] D38982: [refactor] Initial outline of implementation of "extract function" refactoring

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Basic/DiagnosticRefactoringKinds.td:24
+  "not overlap with the AST nodes of interest">;
+
+def err_refactor_code_outside_of_function : Error<"the selected code is not a "

ioeric wrote:
> nit: was this newline intended?
Yeah, it separates the extraction errors from the others.


Repository:
  rL LLVM

https://reviews.llvm.org/D38982



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

r316467 - Add missing clangRewrite lib dependency for clangToolingRefactor

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Oct 24 10:23:53 2017
New Revision: 316467

URL: http://llvm.org/viewvc/llvm-project?rev=316467&view=rev
Log:
Add missing clangRewrite lib dependency for clangToolingRefactor

Modified:
cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt

Modified: cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt?rev=316467&r1=316466&r2=316467&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt (original)
+++ cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt Tue Oct 24 10:23:53 2017
@@ -19,5 +19,6 @@ add_clang_library(clangToolingRefactor
   clangFormat
   clangIndex
   clangLex
+  clangRewrite
   clangToolingCore
   )


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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D39079#905371, @tmsriram wrote:

> In https://reviews.llvm.org/D39079#905353, @hfinkel wrote:
>
> > Noting that, as @vit9696  pointed out in https://reviews.llvm.org/D38554, 
> > this does not suppress uses of the PLT that occur from 
> > backend/optimizer-generated functions (e.g., calls into compiler-rt and 
> > similar).
>
>
> Can I work on this as a follow-up?


I have no objection to that, although adding a mechanism to fix this (which I 
imagine would be an attribute tied to the caller, not the callee, would 
probably end up replacing this mechanism).


https://reviews.llvm.org/D39079



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

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

[Derek Schuff via Phabricator via cfe-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/ToolChain.cpp:318
+  else
+OSLibName = getOS();
   llvm::sys::path::append(Path, "lib", OSLibName);

sbc100 wrote:
> dschuff wrote:
> > Is this logic intended to replace what was removed from CommonArgs.cpp? 
> > Should there be an assert here too?
> Just didn't see the point of that assert.  Can you see what the intention 
> might be?  I don't see why AddRunTimeLibs() should be callable for any/all 
> triples, do you?  I would have had to add  llvm::Triple::Unknown to the list 
> of supported OSs, which seemed strange.
I assume it was just because different OSes have different conventions for the 
rtlib, and if the OS is unknown and/or there's no particular support, then 
there might be a bug. But OTOH it doesn't seem bad to have some reasonable 
default either. For that matter it also seems a little weird that now we are 
letting the binary format be the determining thing for the Compiler-RT path. 
But I guess the real issue is that none of the OS, arch, or binary format is 
really the determining thing; it's really the toolchain/SDK or distribution of 
the compiler that determines what the path should be, and that can be affected 
by a lot of other things (e.g. is it the "system" compiler or not, is it a 
cross compiler, etc). So... yeah I think having this as a default makes as much 
sense as asserting, and when someone needs to add support for their 
distribution, then I suppose it's on them to refactor as needed and keep the 
tests working.


https://reviews.llvm.org/D39218



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Let me phrase it differently. What is this patch (and the matching backend PR) 
supposed to achieve? There are effectively two ways to get rid of PLT entries:
(1) Bind references locally. This is effectively what -Bsymbolic does and what 
is breaking the ELF interposition rules.
(2) Do an indirect call via the GOT. Requires knowing what  an external symbol 
is, making it non-attractive for anything but LTO, since it will create 
performance issues for all non-local accesses (i.e. anything private).


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D39079#905372, @joerg wrote:

> Why again is this a good idea?


It saves the direct jump to the PLT, reducing icache pressure, which is a major 
cost in some workloads.

> This is an even worse hack than -Bsymbolic,

Personally, I would like to build LLVM with -Bsymbolic so that we can build 
LLVM as a DSO and load it from clang without regressing startup time, so I 
don't see what's so terrible about -Bsymbolic, especially for C++ programs.

> the latter at least is visible in ELF header without code inspection. This is 
> breaking core premises of ELF.

What are you talking about?

Anyway, LLVM already has an attribute, nonlazybind, and this just provides a 
flag to apply it to all declarations. It gives the user access to the GOTPCREL 
relocations that we, and loaders, already support.


https://reviews.llvm.org/D39079



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

Re: [PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via cfe-commits]

On Tue, Oct 24, 2017 at 10:25 AM, Joerg Sonnenberger via Phabricator
 wrote:
> joerg added a comment.
>
> Let me phrase it differently. What is this patch (and the matching backend 
> PR) supposed to achieve? There are effectively two ways to get rid of PLT 
> entries:
> (1) Bind references locally. This is effectively what -Bsymbolic does and 
> what is breaking the ELF interposition rules.
> (2) Do an indirect call via the GOT. Requires knowing what  an external 
> symbol is, making it non-attractive for anything but LTO, since it will 
> create performance issues for all non-local accesses (i.e. anything private).

With regards to performance issues, are you saying this knowing that
GOLD and BFD linkers (lld patch will follow) can convert the indirect
calls to direct?   GOLD discussion here:
https://sourceware.org/ml/binutils/2016-05/msg00322.html

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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D39079#905395, @joerg wrote:

> Let me phrase it differently. What is this patch (and the matching backend 
> PR) supposed to achieve? There are effectively two ways to get rid of PLT 
> entries:
>  (1) Bind references locally. This is effectively what -Bsymbolic does and 
> what is breaking the ELF interposition rules.
>  (2) Do an indirect call via the GOT. Requires knowing what  an external 
> symbol is, making it non-attractive for anything but LTO, since it will 
> create performance issues for all non-local accesses (i.e. anything private).


This patch does 2. According to @tmsriram, clever linkers can turn the indirect 
call back into a nop+call_pcrel32. If this isn't universal, the user must know 
what their linker supports. I don't see how it causes performance issues for 
non-local calls, since the PLT will do a jump through the GOT anyway.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

> what is breaking the ELF interposition rules

Frankly, in retrospect, ELF's interposition rules (or at least the defaults for 
those rules), seem suboptimal on several fronts. While breaking them has some 
unfortunate consistency implications, I don't consider breaking them a bad 
thing.


https://reviews.llvm.org/D39079



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

[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Can you remind me why `NamedDecl::printQualifiedName` is not appropriate for 
your needs?


https://reviews.llvm.org/D39224



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

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

[John McCall via Phabricator via cfe-commits]

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

Okay, LGTM.


https://reviews.llvm.org/D39069



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

[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D39138#905184, @hfinkel wrote:

> In https://reviews.llvm.org/D39138#904747, @rjmccall wrote:
>
> > Okay, if this is just for your own checking, I'd rather not take it.  It's 
> > not a significant compile-time cost, but there's no reason to pay it at all.
>
>
> In that case, can we take it? I'd rather have everything decorated for use by 
> the type sanitizer.


There will never, ever be an illegal access to this alloca.


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



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

[PATCH] D39138: [CodeGen] Generate TBAA info for 'this' pointers

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D39138#905445, @rjmccall wrote:

> In https://reviews.llvm.org/D39138#905184, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D39138#904747, @rjmccall wrote:
> >
> > > Okay, if this is just for your own checking, I'd rather not take it.  
> > > It's not a significant compile-time cost, but there's no reason to pay it 
> > > at all.
> >
> >
> > In that case, can we take it? I'd rather have everything decorated for use 
> > by the type sanitizer.
>
>
> There will never, ever be an illegal access to this alloca.


Never mind, then ;)


Repository:
  rL LLVM

https://reviews.llvm.org/D39138



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In https://reviews.llvm.org/D39079#905396, @rnk wrote:

> In https://reviews.llvm.org/D39079#905372, @joerg wrote:
>
> > Why again is this a good idea?
>
>
> It saves the direct jump to the PLT, reducing icache pressure, which is a 
> major cost in some workloads.


It also increases the pressure on the branch predictor, so it is not really 
black and white.

>> This is an even worse hack than -Bsymbolic,
> 
> Personally, I would like to build LLVM with -Bsymbolic so that we can build 
> LLVM as a DSO and load it from clang without regressing startup time, so I 
> don't see what's so terrible about -Bsymbolic, especially for C++ programs.

Qt5 tries that. Requires further hacks as the main binary must be compiled as 
fully position independent code to not run into fun latter. Fun with copy 
relocations is only part of it.

> Anyway, LLVM already has an attribute, nonlazybind, and this just provides a 
> flag to apply it to all declarations. It gives the user access to the 
> GOTPCREL relocations that we, and loaders, already support.

The loader doesn't see GOTPCREL anymore. It also requires a linker that 
disassembles instructions, because it can't distinguish between a normal 
pointer load and a call, to be able to optimize it.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In https://reviews.llvm.org/D39079#905423, @rnk wrote:

> In https://reviews.llvm.org/D39079#905395, @joerg wrote:
>
> > Let me phrase it differently. What is this patch (and the matching backend 
> > PR) supposed to achieve? There are effectively two ways to get rid of PLT 
> > entries:
> >  (1) Bind references locally. This is effectively what -Bsymbolic does and 
> > what is breaking the ELF interposition rules.
> >  (2) Do an indirect call via the GOT. Requires knowing what  an external 
> > symbol is, making it non-attractive for anything but LTO, since it will 
> > create performance issues for all non-local accesses (i.e. anything 
> > private).
>
>
> This patch does 2. According to @tmsriram, clever linkers can turn the 
> indirect call back into a nop+call_pcrel32. If this isn't universal, the user 
> must know what their linker supports. I don't see how it causes performance 
> issues for non-local calls, since the PLT will do a jump through the GOT 
> anyway.


Yes, please see this for GOLD linkers: 
https://sourceware.org/ml/binutils/2016-05/msg00322.html


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In https://reviews.llvm.org/D39079#905454, @joerg wrote:

> In https://reviews.llvm.org/D39079#905396, @rnk wrote:
>
> > In https://reviews.llvm.org/D39079#905372, @joerg wrote:
> >
> > > Why again is this a good idea?
> >
> >
> > It saves the direct jump to the PLT, reducing icache pressure, which is a 
> > major cost in some workloads.
>
>
> It also increases the pressure on the branch predictor, so it is not really 
> black and white.


My experiments show that doing this improves performance of some our large 
workloads by upto 1% and it happens with a reduction in iTLB misses.

> 
> 
>>> This is an even worse hack than -Bsymbolic,
>> 
>> Personally, I would like to build LLVM with -Bsymbolic so that we can build 
>> LLVM as a DSO and load it from clang without regressing startup time, so I 
>> don't see what's so terrible about -Bsymbolic, especially for C++ programs.
> 
> Qt5 tries that. Requires further hacks as the main binary must be compiled as 
> fully position independent code to not run into fun latter. Fun with copy 
> relocations is only part of it.
> 
>> Anyway, LLVM already has an attribute, nonlazybind, and this just provides a 
>> flag to apply it to all declarations. It gives the user access to the 
>> GOTPCREL relocations that we, and loaders, already support.
> 
> The loader doesn't see GOTPCREL anymore. It also requires a linker that 
> disassembles instructions, because it can't distinguish between a normal 
> pointer load and a call, to be able to optimize it.

The linker can replace indirect calls via GOTPCREL with direct calls, both GOLD 
and BFD linker support this today.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D39079#905454, @joerg wrote:

> It also increases the pressure on the branch predictor, so it is not really 
> black and white.


I don't understand this objection. I'm assuming that the PLT stub is an 
indirect jump through the PLTGOT, not a hotpatched stub that jumps directly to 
the definition chosen by the loader. This is the ELF model that I'm familiar 
with, especially since calls to code more than 2GB away generally need to be 
indirect anyway.

> Qt5 tries that. Requires further hacks as the main binary must be compiled as 
> fully position independent code to not run into fun latter. Fun with copy 
> relocations is only part of it.

I'm not sure I understand, but this patch isn't introducing copy relocations, 
to be clear.

> The loader doesn't see GOTPCREL anymore. It also requires a linker that 
> disassembles instructions, because it can't distinguish between a normal 
> pointer load and a call, to be able to optimize it.

Well, yes. The user needs to know that they have an x86-encoding-aware linker, 
or using this flag is probably going to slow their code down. From my 
perspective, this is a performance tuning flag, so that's reasonable.


https://reviews.llvm.org/D39079



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In https://reviews.llvm.org/D39079#905468, @rnk wrote:

> In https://reviews.llvm.org/D39079#905454, @joerg wrote:
>
> > It also increases the pressure on the branch predictor, so it is not really 
> > black and white.
>
>
> I don't understand this objection. I'm assuming that the PLT stub is an 
> indirect jump through the PLTGOT, not a hotpatched stub that jumps directly 
> to the definition chosen by the loader. This is the ELF model that I'm 
> familiar with, especially since calls to code more than 2GB away generally 
> need to be indirect anyway.


Yes, this is correct.  A PLT stub for x86_64 looks like this:

  jmpq   *0x2ada(%rip)# 403000 <_GLOBAL_OFFSET_TABLE_+0x18>
  pushq  $0x0
  jmpq   400510 <_init+0x30>

It has three instructions and the last two are only useful if lazy binding is 
done. With early binding, the last two instructions is dead code. What this 
patch does is to take that first instruction and put it at the point where the 
call is made to the PLT, that's it.  Really, with early binding, the PLT  stub 
is a completely redundant piece of code.  I can't see how you argue with this.

> 
> 
>> Qt5 tries that. Requires further hacks as the main binary must be compiled 
>> as fully position independent code to not run into fun latter. Fun with copy 
>> relocations is only part of it.
> 
> I'm not sure I understand, but this patch isn't introducing copy relocations, 
> to be clear.
> 
>> The loader doesn't see GOTPCREL anymore. It also requires a linker that 
>> disassembles instructions, because it can't distinguish between a normal 
>> pointer load and a call, to be able to optimize it.
> 
> Well, yes. The user needs to know that they have an x86-encoding-aware 
> linker, or using this flag is probably going to slow their code down. From my 
> perspective, this is a performance tuning flag, so that's reasonable.




https://reviews.llvm.org/D39079



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

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

You

In https://reviews.llvm.org/D38985#901152, @sammccall wrote:

> Hi Alex! I'm working on clangd, but am pretty new to the project, so forgive 
> some naive questions.
>
> I'm a bit unclear at a high level what the new abstractions in this patch 
> add, in terms of wiring refactorings up to clangd and other tools.
>  My understanding is: we have a hierarchy of refactorings consisting of 
> actions (roughly: user intent?) and rules (roughly: implementations).
>
> Clangd needs to be able to:
>
> - enumerate the actions and their associated rules: possible via 
> `createRefactoringActions()`
> - find names for these (machine readable and human readable): possible at the 
> `Action` level, not `Rule` yet
> - determine whether rules can be applied and what configuration is needed: 
> possible via the `Rule` API
> - invoke rules: possible via the `Rule` API


Right.

> So AFAICT, clangd could be hooked up to the existing Refactor API, without 
> `EditorCommand` or `EditorClient`. What's the gain?



- EditorCommand is not a necessity, but an abstraction to keep the rule simple. 
I thought it'd be good to have them in a registry for some libclang uses 
(particularly mapping to C API enums), but on a second though I think that this 
can be avoided. I'll avoid the class altogether then.
- EditorClient simplifies the editor operation and will be reused in libclang.


Repository:
  rL LLVM

https://reviews.llvm.org/D38985



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

[PATCH] D39064: implement __has_unique_object_representations

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 120106.
erichkeane added a comment.

Based on @rnk s comments, I discovered that base-class handling is more 
difficult than I suspected!  I added a couple more tests and am properly 
handling base classes.


https://reviews.llvm.org/D39064

Files:
  include/clang/AST/Type.h
  include/clang/Basic/TokenKinds.def
  include/clang/Basic/TypeTraits.h
  lib/AST/Type.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -716,6 +716,7 @@
 ///   '__is_sealed'   [MS]
 ///   '__is_trivial'
 ///   '__is_union'
+///   '__has_unique_object_representations'
 ///
 /// [Clang] unary-type-trait:
 ///   '__is_aggregate'
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2166,6 +2166,152 @@
   return false;
 }
 
+bool QualType::unionHasUniqueObjectRepresentations(
+const ASTContext &Context) const {
+  assert((*this)->isUnionType() && "must be union type");
+  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
+  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
+
+  for (const auto *Field : Union->fields()) {
+if (!Field->getType().hasUniqueObjectRepresentations(Context))
+  return false;
+CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
+if (FieldSize != UnionSize)
+  return false;
+  }
+  return true;
+}
+
+bool isStructEmpty(QualType Ty) {
+  assert(Ty.getTypePtr()->isStructureOrClassType() &&
+ "Must be struct or class");
+  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
+
+  if (!RD->field_empty())
+return false;
+
+  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
+return ClassDecl->isEmpty();
+  }
+
+  return true;
+}
+
+bool QualType::structHasUniqueObjectRepresentations(
+const ASTContext &Context) const {
+  assert((*this)->isStructureOrClassType() && "Must be struct or class");
+  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
+
+  if (isStructEmpty(*this))
+return false;
+
+  // Check base types.
+  CharUnits BaseSize{};
+  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
+for (const auto Base : ClassDecl->bases()) {
+  if (Base.isVirtual())
+return false;
+
+  // Empty bases are permitted, otherwise ensure base has unique
+  // representation. Also, Empty Base Optimization means that an
+  // Empty base takes up 0 size.
+  if (!isStructEmpty(Base.getType())) {
+if (!Base.getType().structHasUniqueObjectRepresentations(Context))
+  return false;
+BaseSize += Context.getTypeSizeInChars(Base.getType());
+  }
+}
+  }
+
+  CharUnits StructSize = Context.getTypeSizeInChars(*this);
+
+  // This struct obviously has bases that keep it from being 'empty', so
+  // checking fields is no longer required.  Ensure that the struct size
+  // is the sum of the bases.
+  if (RD->field_empty())
+return StructSize == BaseSize;
+  ;
+
+  CharUnits CurOffset =
+  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
+
+  // If the first field isn't at the sum of the size of the bases, there
+  // is padding somewhere.
+  if (BaseSize != CurOffset)
+return false;
+
+  for (const auto *Field : RD->fields()) {
+if (!Field->getType().hasUniqueObjectRepresentations(Context))
+  return false;
+CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
+CharUnits FieldOffset =
+Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
+// Has padding between fields.
+if (FieldOffset != CurOffset)
+  return false;
+CurOffset += FieldSize;
+  }
+  // Check for tail padding.
+  return CurOffset == StructSize;
+}
+
+bool QualType::hasUniqueObjectRepresentations(const ASTContext &Context) const {
+  // C++17 [meta.unary.prop]:
+  //   The predicate condition for a template specialization
+  //   has_unique_object_representations shall be
+  //   satisfied if and only if:
+  // (9.1) — T is trivially copyable, and
+  // (9.2) — any two objects of type T with the same value have the same
+  // object representation, where two objects
+  //   of array or non-union class type are considered to have the same value
+  //   if their respective sequences of
+  //   direct subobjects have the same values, and two objects of union type
+  //   are considered to have the same
+  //   value if they have the same active member and the corresponding members
+  //   have the same value.
+  //   The set of scalar types for which this condition holds is
+  //   implementation-defined. [ Note: If a type has padding
+  //   bits, the condition does not hold; otherwise, the condi

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

In https://reviews.llvm.org/D39079#905468, @rnk wrote:

> In https://reviews.llvm.org/D39079#905454, @joerg wrote:
>
> > It also increases the pressure on the branch predictor, so it is not really 
> > black and white.
>
>
> I don't understand this objection. I'm assuming that the PLT stub is an 
> indirect jump through the PLTGOT,
>  not a hotpatched stub that jumps directly to the definition chosen by the 
> loader. This is the ELF model
>  that I'm familiar with, especially since calls to code more than 2GB away 
> generally need to be indirect anyway.


Correct, so all local calls to the same function go via the same location and 
share the predication of the indirect jump.

>> Qt5 tries that. Requires further hacks as the main binary must be compiled 
>> as fully position independent code to not run into fun latter. Fun with copy 
>> relocations is only part of it.
> 
> I'm not sure I understand, but this patch isn't introducing copy relocations, 
> to be clear.

That was in reference to using it for clang.


https://reviews.llvm.org/D39079



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

[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov updated this revision to Diff 120109.

https://reviews.llvm.org/D39220

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  lib/Analysis/AnalysisDeclContext.cpp


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -306,8 +306,8 @@
 
 BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
   if (!BdyFrm)
-BdyFrm = new BodyFarm(ASTCtx, Injector.get());
-  return BdyFrm;
+BdyFrm = llvm::make_unique(ASTCtx, Injector.get());
+  return BdyFrm.get();
 }
 
 const StackFrameContext *
@@ -603,11 +603,6 @@
   }
 }
 
-AnalysisDeclContextManager::~AnalysisDeclContextManager() {
-  if (BdyFrm)
-delete BdyFrm;
-}
-
 LocationContext::~LocationContext() {}
 
 LocationContextManager::~LocationContextManager() {
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -421,7 +421,7 @@
 
   /// Pointer to a factory for creating and caching implementations for common
   /// methods during the analysis.
-  BodyFarm *BdyFrm = nullptr;
+  std::unique_ptr BdyFrm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
@@ -438,8 +438,6 @@
  bool addCXXNewAllocator = true,
  CodeInjector *injector = nullptr);
 
-  ~AnalysisDeclContextManager();
-
   AnalysisDeclContext *getContext(const Decl *D);
 
   bool getUseUnoptimizedCFG() const {


Index: lib/Analysis/AnalysisDeclContext.cpp
===
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -306,8 +306,8 @@
 
 BodyFarm *AnalysisDeclContextManager::getBodyFarm() {
   if (!BdyFrm)
-BdyFrm = new BodyFarm(ASTCtx, Injector.get());
-  return BdyFrm;
+BdyFrm = llvm::make_unique(ASTCtx, Injector.get());
+  return BdyFrm.get();
 }
 
 const StackFrameContext *
@@ -603,11 +603,6 @@
   }
 }
 
-AnalysisDeclContextManager::~AnalysisDeclContextManager() {
-  if (BdyFrm)
-delete BdyFrm;
-}
-
 LocationContext::~LocationContext() {}
 
 LocationContextManager::~LocationContextManager() {
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -421,7 +421,7 @@
 
   /// Pointer to a factory for creating and caching implementations for common
   /// methods during the analysis.
-  BodyFarm *BdyFrm = nullptr;
+  std::unique_ptr BdyFrm;
 
   /// Flag to indicate whether or not bodies should be synthesized
   /// for well-known functions.
@@ -438,8 +438,6 @@
  bool addCXXNewAllocator = true,
  CodeInjector *injector = nullptr);
 
-  ~AnalysisDeclContextManager();
-
   AnalysisDeclContext *getContext(const Decl *D);
 
   bool getUseUnoptimizedCFG() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39251: [libunwind] Fix building for ARM with dwarf exception handling

[Martin Storsjö via Phabricator via cfe-commits]

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

The previous definition of _LIBUNWIND_HIGHEST_DWARF_REGISTER seems to be a copy 
of the ARM64 value (introduced in SVN r276128); since the code actually hasn't 
compiled properly for arm in dwarf mode before, this hasn't actually been used. 
Set it to the correct value based on the UNW_ARM_* enum values.

Use the same size for data types unw_word_t as for _LIBUNWIND_ARM_EHABI, to 
have the struct sizes match.

This requires adding casts in a log line, where the data types (unw_word_t) 
implicitly were assumed be uint64_t. Normally mismatched
printf formats aren't fatal (and might go unnoticed), unless a later parameter 
is interpreted as a string, when it becomes fatal.


https://reviews.llvm.org/D39251

Files:
  include/__libunwind_config.h
  include/libunwind.h
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/UnwindLevel1.c
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -55,7 +55,7 @@
 # define REGISTER_KIND Registers_ppc
 #elif defined(__aarch64__)
 # define REGISTER_KIND Registers_arm64
-#elif defined(_LIBUNWIND_ARM_EHABI)
+#elif defined(__arm__)
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -78,8 +78,8 @@
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase1(ex_ojb=%p): pc=0x%" PRIx64 ", start_ip=0x%" PRIx64
   ", func=%s, lsda=0x%" PRIx64 ", personality=0x%" PRIx64 "",
-  (void *)exception_object, pc, frameInfo.start_ip, functionName,
-  frameInfo.lsda, frameInfo.handler);
+  (void *)exception_object, (uint64_t)pc, (uint64_t)frameInfo.start_ip, functionName,
+  (uint64_t)frameInfo.lsda, (uint64_t)frameInfo.handler);
 }
 
 // If there is a personality routine, ask it if it will want to stop at
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -583,6 +583,12 @@
   }
 #endif
 
+#if defined(_LIBUNWIND_TARGET_ARM)
+  compact_unwind_encoding_t dwarfEncoding(Registers_arm &) const {
+return 0;
+  }
+#endif
+
 #if defined (_LIBUNWIND_TARGET_OR1K)
   compact_unwind_encoding_t dwarfEncoding(Registers_or1k &) const {
 return 0;
Index: src/Registers.hpp
===
--- src/Registers.hpp
+++ src/Registers.hpp
@@ -1326,7 +1326,7 @@
   Registers_arm(const void *registers);
 
   boolvalidRegister(int num) const;
-  uint32_tgetRegister(int num);
+  uint32_tgetRegister(int num) const;
   voidsetRegister(int num, uint32_t value);
   boolvalidFloatRegister(int num) const;
   unw_fpreg_t getFloatRegister(int num);
@@ -1339,6 +1339,7 @@
 restoreSavedFloatRegisters();
 restoreCoreAndJumpTo();
   }
+  static int  lastDwarfRegNum() { return 287; }
 
   uint32_t  getSP() const { return _registers.__sp; }
   void  setSP(uint32_t value) { _registers.__sp = value; }
@@ -1473,7 +1474,7 @@
   return false;
 }
 
-inline uint32_t Registers_arm::getRegister(int regNum) {
+inline uint32_t Registers_arm::getRegister(int regNum) const {
   if (regNum == UNW_REG_SP || regNum == UNW_ARM_SP)
 return _registers.__sp;
 
Index: include/libunwind.h
===
--- include/libunwind.h
+++ include/libunwind.h
@@ -72,7 +72,7 @@
 typedef struct unw_addr_space *unw_addr_space_t;
 
 typedef int unw_regnum_t;
-#if defined(_LIBUNWIND_ARM_EHABI)
+#if defined(_LIBUNWIND_ARM_EHABI) || defined(__ARM_DWARF_EH__)
 typedef uint32_t unw_word_t;
 typedef uint64_t unw_fpreg_t;
 #else
Index: include/__libunwind_config.h
===
--- include/__libunwind_config.h
+++ include/__libunwind_config.h
@@ -45,7 +45,7 @@
 #define _LIBUNWIND_CONTEXT_SIZE 42
 #define _LIBUNWIND_CURSOR_SIZE 49
 #  endif
-#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 96
+#  define _LIBUNWIND_HIGHEST_DWARF_REGISTER 288
 # elif defined(__or1k__)
 #  define _LIBUNWIND_TARGET_OR1K 1
 #  define _LIBUNWIND_CONTEXT_SIZE 16
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39218: [WebAssembly] Include libclang_rt.builtins in the standard way

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 120111.
sbc100 added a comment.

- remove debugging
- git squash commit for startup_libs.
- update test expectations


https://reviews.llvm.org/D39218

Files:
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  test/Driver/wasm-toolchain.c


Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -27,10 +27,10 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "[[temp]]" 
"-allow-undefined-file" "wasm.syms" "-lc" "-lcompiler_rt" "-o" "a.out"
+// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" 
"-allow-undefined-file" "wasm.syms" "-lc" 
"{{.*}}/libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization.
 
 // RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "[[temp]]" 
"-allow-undefined-file" "wasm.syms" "-lc" "-lcompiler_rt" "-o" "a.out"
+// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" 
"-allow-undefined-file" "wasm.syms" "-lc" 
"{{.*}}/libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -49,6 +49,9 @@
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
+CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt1.o")));
+
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -61,7 +64,7 @@
 CmdArgs.push_back("-allow-undefined-file");
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("wasm.syms")));
 CmdArgs.push_back("-lc");
-CmdArgs.push_back("-lcompiler_rt");
+AddRunTimeLibs(ToolChain, ToolChain.getDriver(), CmdArgs, Args);
   }
 
   CmdArgs.push_back("-o");
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -1027,15 +1027,7 @@
 
   switch (RLT) {
   case ToolChain::RLT_CompilerRT:
-switch (TC.getTriple().getOS()) {
-default:
-  llvm_unreachable("unsupported OS");
-case llvm::Triple::Win32:
-case llvm::Triple::Linux:
-case llvm::Triple::Fuchsia:
-  CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
-  break;
-}
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
 break;
   case ToolChain::RLT_Libgcc:
 // Make sure libgcc is not used under MSVC environment by default
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -309,8 +309,12 @@
 
 std::string ToolChain::getCompilerRTPath() const {
   SmallString<128> Path(getDriver().ResourceDir);
-  StringRef OSLibName = Triple.isOSFreeBSD() ? "freebsd" : getOS();
-  llvm::sys::path::append(Path, "lib", OSLibName);
+  if (Triple.isOSUnknown()) {
+llvm::sys::path::append(Path, "lib");
+  } else {
+StringRef OSLibName = Triple.isOSFreeBSD() ? "freebsd" : getOS();
+llvm::sys::path::append(Path, "lib", OSLibName);
+  }
   return Path.str();
 }
 


Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -27,10 +27,10 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "-lcompiler_rt" "-o" "a.out"
+// LINK: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "crt1.o" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "{{.*}}/libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization.
 
 // RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT: lld{{.*}}" "-flavor" "wasm" "-L/foo/lib" "[[temp]]" "-allow-undefined-file" "wasm.syms" "-lc" "-lcompiler_rt" "-o" "a.out"
+// LINK_OPT: lld{{.*}}" "-fl

r316484 - CodeGen: Fix missing debug loc due to alloca

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Oct 24 12:14:43 2017
New Revision: 316484

URL: http://llvm.org/viewvc/llvm-project?rev=316484&view=rev
Log:
CodeGen: Fix missing debug loc due to alloca

Builder save/restores insertion pointer when emitting addr space cast
for alloca, but does not save/restore debug loc, which causes verifier
failure for certain call instructions.

This patch fixes that.

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

Added:
cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=316484&r1=316483&r2=316484&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Oct 24 12:14:43 2017
@@ -74,12 +74,11 @@ Address CodeGenFunction::CreateTempAlloc
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);

Added: cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl?rev=316484&view=auto
==
--- cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl (added)
+++ cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl Tue Oct 24 12:14:43 2017
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 
-emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg ![[LOC:[0-9]+]]
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg ![[LOC]]
+func2(func1());
+}
+


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

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316484: CodeGen: Fix missing debug loc due to alloca 
(authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D39069?vs=120076&id=120115#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39069

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -74,12 +74,11 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
Index: cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
+++ cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 
-emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg ![[LOC:[0-9]+]]
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg ![[LOC]]
+func2(func1());
+}
+


Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -74,12 +74,11 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) {
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
Index: cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
+++ cfe/trunk/test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 -emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg ![[LOC:[0-9]+]]
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg ![[LOC]]
+func2(func1());
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39064: implement __has_unique_object_representations

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/AST/Type.cpp:2212-2213
+for (const auto Base : ClassDecl->bases()) {
+  if (Base.isVirtual())
+return false;
+

OK, I guess vbases don't have a unique representation. :) What about vtable 
slots? Should we check isDynamic?


https://reviews.llvm.org/D39064



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

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I'd still _very_ much like a solution that is acceptable for all libc to use, 
and have that on by default.

However, I guess this is fine.

Only concern I have is that it seems odd that so many test-cases needed to be 
changed. What fails without those test changes?


https://reviews.llvm.org/D34158



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

r316488 - [OPENMP] Fix PR35013: Fix passing VLAs captures to outlined functions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Oct 24 12:52:31 2017
New Revision: 316488

URL: http://llvm.org/viewvc/llvm-project?rev=316488&view=rev
Log:
[OPENMP] Fix PR35013: Fix passing VLAs captures to outlined functions.

Fixed passing of VLAs and variably-modified types to outlined functions.
Synchronized passing with the types codegen.

Modified:
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
cfe/trunk/test/OpenMP/parallel_codegen.cpp
cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_codegen.cpp
cfe/trunk/test/OpenMP/vla_crash.c

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=316488&r1=316487&r2=316488&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Tue Oct 24 12:52:31 2017
@@ -254,6 +254,12 @@ static QualType getCanonicalParamType(AS
   }
   if (T->isPointerType())
 return C.getPointerType(getCanonicalParamType(C, T->getPointeeType()));
+  if (auto *A = T->getAsArrayTypeUnsafe()) {
+if (auto *VLA = dyn_cast(A))
+  return getCanonicalParamType(C, VLA->getElementType());
+else if (!A->isVariablyModifiedType())
+  return C.getCanonicalType(T);
+  }
   return C.getCanonicalParamType(T);
 }
 
@@ -327,7 +333,7 @@ static llvm::Function *emitOutlinedFunct
   II = &Ctx.Idents.get("vla");
 }
 if (ArgType->isVariablyModifiedType())
-  ArgType = getCanonicalParamType(Ctx, ArgType.getNonReferenceType());
+  ArgType = getCanonicalParamType(Ctx, ArgType);
 auto *Arg =
 ImplicitParamDecl::Create(Ctx, /*DC=*/nullptr, FD->getLocation(), II,
   ArgType, ImplicitParamDecl::Other);

Modified: cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_reduction_codegen.cpp?rev=316488&r1=316487&r2=316488&view=diff
==
--- cfe/trunk/test/OpenMP/for_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_reduction_codegen.cpp Tue Oct 24 12:52:31 2017
@@ -524,7 +524,7 @@ int main() {
 // CHECK: store float [[UP]], float* [[T_VAR1_LHS]],
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* 
%{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* 
dereferenceable(160) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* 
{{.+}} %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x 
[[S_FLOAT_TY* dereferenceable(160) %{{.+}})
 
 // Reduction list for runtime.
 // CHECK: [[RED_LIST:%.+]] = alloca [4 x i8*],
@@ -725,7 +725,7 @@ int main() {
 
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* 
%{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* 
{{.+}} %{{.+}}, [10 x [4 x [[S_FLOAT_TY* dereferenceable(160) %{{.+}})
 
 // CHECK: [[ARRS_PRIV:%.+]] = alloca [10 x [4 x [[S_FLOAT_TY,
 

Modified: cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp?rev=316488&r1=316487&r2=316488&view=diff
==
--- cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp (original)
+++ cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp Tue Oct 24 12:52:31 2017
@@ -307,7 +307,7 @@ int main() {
 // CHECK: fadd float 5.55e+02, %
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* 
%{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x [[S_FLOAT_TY* 
dereferenceable(480) %{{.+}})
+// CHECK: define internal void [[MAIN_MICROTASK1]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* 
{{.+}} %{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [10 x [4 x 
[[S_FLOAT_TY* dereferenceable(480) %{{.+}})
 
 // Reduction list for runtime.
 // CHECK: [[RED_LIST:%.+]] = alloca [4 x i8*],
@@ -503,7 +503,7 @@ int main() {
 
 // CHECK: ret void
 
-// CHECK: define internal void [[MAIN_MICROTASK2]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-

[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr

[Zhihao Yuan via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D39220



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

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In https://reviews.llvm.org/D34158#905596, @jyknight wrote:

> I'd still _very_ much like a solution that is acceptable for all libc to use, 
> and have that on by default.
>
> However, I guess this is fine.
>
> Only concern I have is that it seems odd that so many test-cases needed to be 
> changed. What fails without those test changes?


Those tests fail because they generate preprocessed output and then check 
carefully for precise results. Since the preprocessed output is different, the 
comparison fails. I "fixed" the tests by adding the freestanding option which 
inhibits the new behavior.  I'll have to check out and rebuild everything so I 
can show you exactly the failure mode, I expect I can post details tomorrow.


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] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Have you tried this change against the GDB and LLDB test suites?  If they have 
failures then we should think about whether those tests are over-specific or 
whether we should put this under a tuning option.


https://reviews.llvm.org/D39239



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

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

[Alex Lorenz via Phabricator via cfe-commits]

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

Avoid using a separate EditorCommand class.


Repository:
  rL LLVM

https://reviews.llvm.org/D38985

Files:
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/EditorClient.cpp
  lib/Tooling/Refactoring/Extract.cpp
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -10,6 +10,7 @@
 #include "ReplacementTest.h"
 #include "RewriterTestContext.h"
 #include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/RefactoringAction.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRules.h"
 #include "clang/Tooling/Refactoring/RefactoringDiagnostic.h"
 #include "clang/Tooling/Refactoring/Rename/SymbolName.h"
@@ -219,4 +220,24 @@
 SourceRange(Cursor, Cursor.getLocWithOffset(strlen("test";
 }
 
+TEST_F(RefactoringActionRulesTest, EditorCommandBinding) {
+  std::vector> Actions =
+  createRefactoringActions();
+  for (auto &Action : Actions) {
+if (Action->getCommand() == "extract") {
+  std::vector> Rules =
+  Action->createActiveActionRules();
+  ASSERT_FALSE(Rules.empty());
+  Optional> Cmd =
+  Rules[0]->getEditorCommandInfo();
+  ASSERT_TRUE(Cmd);
+  EXPECT_EQ(Cmd->first, "extract-function");
+  EXPECT_EQ(Cmd->second, "Extract Function");
+  return;
+}
+  }
+  // Never found 'extract'?
+  ASSERT_TRUE(false);
+}
+
 } // end anonymous namespace
Index: lib/Tooling/Refactoring/Extract.cpp
===
--- lib/Tooling/Refactoring/Extract.cpp
+++ lib/Tooling/Refactoring/Extract.cpp
@@ -215,9 +215,11 @@
   /// action.
   RefactoringActionRules createActionRules() const override {
 RefactoringActionRules Rules;
-Rules.push_back(createRefactoringActionRule(
-ExtractableCodeSelectionRequirement(),
-OptionRequirement()));
+Rules.push_back(
+createEditorBinding(createRefactoringActionRule(
+ExtractableCodeSelectionRequirement(),
+OptionRequirement()),
+"extract-function", "Extract Function"));
 return Rules;
   }
 };
Index: lib/Tooling/Refactoring/EditorClient.cpp
===
--- /dev/null
+++ lib/Tooling/Refactoring/EditorClient.cpp
@@ -0,0 +1,52 @@
+//===--- EditorClient.cpp - Refactoring editor integration support ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
+
+namespace clang {
+namespace tooling {
+
+std::unique_ptr
+createEditorBinding(std::unique_ptr Rule, StringRef Name,
+StringRef Title) {
+  class BoundEditorRefactoringActionRule : public RefactoringActionRule {
+  public:
+BoundEditorRefactoringActionRule(
+std::unique_ptr Rule, StringRef Name,
+StringRef Title)
+: Rule(std::move(Rule)), Name(Name), Title(Title) {}
+
+void invoke(RefactoringResultConsumer &Consumer,
+RefactoringRuleContext &Context) override {
+  Rule->invoke(Consumer, Context);
+}
+
+bool hasSelectionRequirement() override {
+  return Rule->hasSelectionRequirement();
+}
+
+void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) override {
+  return Rule->visitRefactoringOptions(Visitor);
+}
+
+Optional> getEditorCommandInfo() override {
+  return std::make_pair(Name, Title);
+}
+
+  private:
+std::unique_ptr Rule;
+StringRef Name;
+StringRef Title;
+  };
+  return llvm::make_unique(std::move(Rule),
+ Name, Title);
+}
+
+} // end namespace tooling
+} // end namespace clang
Index: lib/Tooling/Refactoring/CMakeLists.txt
===
--- lib/Tooling/Refactoring/CMakeLists.txt
+++ lib/Tooling/Refactoring/CMakeLists.txt
@@ -4,6 +4,7 @@
   ASTSelection.cpp
   ASTSelectionRequirements.cpp
   AtomicChange.cpp
+  EditorClient.cpp
   Extract.cpp
   RefactoringActions.cpp
   Rename/RenamingAction.cpp
Index: include/clang/Tooling/Refactoring/RefactoringActionRules.h
===
--- include/clang/Tooling/Refactoring/RefactoringActionRules.h
+++ include/clang/Tooling/Refactoring/RefactoringActionRules.h
@

[PATCH] D39217: [libclang, bindings]: add spelling location

[Johann Klähn via Phabricator via cfe-commits]

jklaehn added inline comments.



Comment at: test/Index/annotate-tokens.c:226
 // CHECK-RANGE2: Punctuation: "." [55:5 - 55:6] UnexposedExpr=
-// CHECK-RANGE2: Identifier: "z" [55:6 - 55:7] MemberRef=z:52:1
+// CHECK-RANGE2: Identifier: "z" [55:6 - 55:7] MemberRef=z:41:9
 // CHECK-RANGE2: Punctuation: "=" [55:8 - 55:9] UnexposedExpr=

Those look like an improvement to me since the `MemberRef`s of `y` and `z` no 
longer refer to the same line?



Comment at: test/Index/c-index-getCursor-test.m:167
 // CHECK: [57:1 - 57:10] FunctionDecl=f:57:6 (Definition)
-// CHECK: [58:4 - 58:8] VarDecl=my_var:58:8 (Definition)
+// CHECK: [58:4 - 58:8] VarDecl=my_var:2:1 (Definition)
 // CHECK: [58:8 - 58:15] macro expansion=CONCAT:55:9

This does not seem to refer to a valid location? (line 2 only contains a 
comment)


https://reviews.llvm.org/D39217



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

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 120129.
arphaman marked an inline comment as done.
arphaman added a comment.

Fix comment nit.


Repository:
  rL LLVM

https://reviews.llvm.org/D38985

Files:
  include/clang/Tooling/Refactoring/RefactoringActionRule.h
  include/clang/Tooling/Refactoring/RefactoringActionRules.h
  lib/Tooling/Refactoring/CMakeLists.txt
  lib/Tooling/Refactoring/EditorClient.cpp
  lib/Tooling/Refactoring/Extract.cpp
  unittests/Tooling/RefactoringActionRulesTest.cpp

Index: unittests/Tooling/RefactoringActionRulesTest.cpp
===
--- unittests/Tooling/RefactoringActionRulesTest.cpp
+++ unittests/Tooling/RefactoringActionRulesTest.cpp
@@ -10,6 +10,7 @@
 #include "ReplacementTest.h"
 #include "RewriterTestContext.h"
 #include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/RefactoringAction.h"
 #include "clang/Tooling/Refactoring/RefactoringActionRules.h"
 #include "clang/Tooling/Refactoring/RefactoringDiagnostic.h"
 #include "clang/Tooling/Refactoring/Rename/SymbolName.h"
@@ -219,4 +220,24 @@
 SourceRange(Cursor, Cursor.getLocWithOffset(strlen("test";
 }
 
+TEST_F(RefactoringActionRulesTest, EditorCommandBinding) {
+  std::vector> Actions =
+  createRefactoringActions();
+  for (auto &Action : Actions) {
+if (Action->getCommand() == "extract") {
+  std::vector> Rules =
+  Action->createActiveActionRules();
+  ASSERT_FALSE(Rules.empty());
+  Optional> Cmd =
+  Rules[0]->getEditorCommandInfo();
+  ASSERT_TRUE(Cmd);
+  EXPECT_EQ(Cmd->first, "extract-function");
+  EXPECT_EQ(Cmd->second, "Extract Function");
+  return;
+}
+  }
+  // Never found 'extract'?
+  ASSERT_TRUE(false);
+}
+
 } // end anonymous namespace
Index: lib/Tooling/Refactoring/Extract.cpp
===
--- lib/Tooling/Refactoring/Extract.cpp
+++ lib/Tooling/Refactoring/Extract.cpp
@@ -215,9 +215,11 @@
   /// action.
   RefactoringActionRules createActionRules() const override {
 RefactoringActionRules Rules;
-Rules.push_back(createRefactoringActionRule(
-ExtractableCodeSelectionRequirement(),
-OptionRequirement()));
+Rules.push_back(
+createEditorBinding(createRefactoringActionRule(
+ExtractableCodeSelectionRequirement(),
+OptionRequirement()),
+"extract-function", "Extract Function"));
 return Rules;
   }
 };
Index: lib/Tooling/Refactoring/EditorClient.cpp
===
--- /dev/null
+++ lib/Tooling/Refactoring/EditorClient.cpp
@@ -0,0 +1,52 @@
+//===--- EditorClient.cpp - Refactoring editor integration support ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/RefactoringActionRules.h"
+
+namespace clang {
+namespace tooling {
+
+std::unique_ptr
+createEditorBinding(std::unique_ptr Rule, StringRef Name,
+StringRef Title) {
+  class BoundEditorRefactoringActionRule : public RefactoringActionRule {
+  public:
+BoundEditorRefactoringActionRule(
+std::unique_ptr Rule, StringRef Name,
+StringRef Title)
+: Rule(std::move(Rule)), Name(Name), Title(Title) {}
+
+void invoke(RefactoringResultConsumer &Consumer,
+RefactoringRuleContext &Context) override {
+  Rule->invoke(Consumer, Context);
+}
+
+bool hasSelectionRequirement() override {
+  return Rule->hasSelectionRequirement();
+}
+
+void visitRefactoringOptions(RefactoringOptionVisitor &Visitor) override {
+  return Rule->visitRefactoringOptions(Visitor);
+}
+
+Optional> getEditorCommandInfo() override {
+  return std::make_pair(Name, Title);
+}
+
+  private:
+std::unique_ptr Rule;
+StringRef Name;
+StringRef Title;
+  };
+  return llvm::make_unique(std::move(Rule),
+ Name, Title);
+}
+
+} // end namespace tooling
+} // end namespace clang
Index: lib/Tooling/Refactoring/CMakeLists.txt
===
--- lib/Tooling/Refactoring/CMakeLists.txt
+++ lib/Tooling/Refactoring/CMakeLists.txt
@@ -4,6 +4,7 @@
   ASTSelection.cpp
   ASTSelectionRequirements.cpp
   AtomicChange.cpp
+  EditorClient.cpp
   Extract.cpp
   RefactoringActions.cpp
   Rename/RenamingAction.cpp
Index: include/clang/Tooling/Refactoring/RefactoringActionRules.h
===
--- include/clang/Tooling/Refactoring/RefactoringActionRules.h
+++ include/clang/Tooling/Refactoring/Refactorin

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In https://reviews.llvm.org/D39079#905519, @joerg wrote:

> In https://reviews.llvm.org/D39079#905468, @rnk wrote:
>
> > In https://reviews.llvm.org/D39079#905454, @joerg wrote:
> >
> > > It also increases the pressure on the branch predictor, so it is not 
> > > really black and white.
> >
> >
> > I don't understand this objection. I'm assuming that the PLT stub is an 
> > indirect jump through the PLTGOT,
> >  not a hotpatched stub that jumps directly to the definition chosen by the 
> > loader. This is the ELF model
> >  that I'm familiar with, especially since calls to code more than 2GB away 
> > generally need to be indirect anyway.
>
>
> Correct, so all local calls to the same function go via the same location and 
> share the predication of the indirect jump.


Weigh this against a .plt that is far away from the actual calls, and is 
causing itlb pressure and is clearly improving itlb behavior by eliminating it 
on our larger workloads.  This is an optimization feature and if this does not 
improve performance in your case, do not enable it.

> 
> 
>>> Qt5 tries that. Requires further hacks as the main binary must be compiled 
>>> as fully position independent code to not run into fun latter. Fun with 
>>> copy relocations is only part of it.
>> 
>> I'm not sure I understand, but this patch isn't introducing copy 
>> relocations, to be clear.
> 
> That was in reference to using it for clang.




https://reviews.llvm.org/D39079



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

[PATCH] D38985: [refactor] Add support for editor commands that connect IDEs/editors to the refactoring actions

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: include/clang/Tooling/Refactoring/RefactoringActionRule.h:58
+  /// Returns the editor command that's was bound to this rule.
+  virtual const EditorCommand *getEditorCommand() { return nullptr; }
 };

ioeric wrote:
> arphaman wrote:
> > ioeric wrote:
> > > I'm still not quite sure about the intention of `EditorCommand` (is it 
> > > supposed to be used as a mapping from name to rule, or the other way 
> > > around?), but I'm a bit concerned about mixing editor logic with the 
> > > refactoring rules this way. Also to enable a editor command, it seems to 
> > > me that we would need to touch both the registry and the rule creation 
> > > code, which seems a bit cumbersome.
> > > 
> > > I think we should either 1) separate out the command logic cleanly 
> > > without touching action/rule interfaces in the refactoring engine or 2) 
> > > simply bake the logic into the refactoring engine.
> > > 
> > > It is unclear to me if 1) is possible, but for 2), I think we could 
> > > introduce a `RefactoringEngine` class which carries all refactoring 
> > > actions as well as a map to serve the purpose of `EditorCommand`. And I 
> > > think by doing this, we could also make the interfaces of 
> > > `RefactoringEngine` more explicit.
> > (Quick comment before the devmeeting:)
> > Mapping from name to rule.
> > You're right though, It might be better to explore an alternative solution, 
> > but I don't quite follow your proposal yet. I'll be in the tooling hacker's 
> > lab at the dev meeting today if you want to discuss this in person.
> Currently, we have `createRefactoringActions` as the API of the refactoring 
> engine/library.  I think we could introduce a `RefactoringEngine` class that 
> exposes all user-facing interfaces from the engine, including 
> creating/accessing actions and providing the mapping of editor command. And 
> the command registration could be handled inside the class, which I assume 
> would be simpler. This would also allow us to expose more interfaces in the 
> future via this class.
> 
> (Sorry, I didn't make it to the dev meeting this year... I hope I could get a 
> chance to meet you in the next dev meeting or the European dev meeting early 
> next year :)
I think that's a good idea.

I will post a follow up patch that moves the rule creation to the engine. This 
one still creates the editor binding in the file.


Repository:
  rL LLVM

https://reviews.llvm.org/D38985



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

[PATCH] D39064: implement __has_unique_object_representations

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: lib/AST/Type.cpp:2212-2213
+for (const auto Base : ClassDecl->bases()) {
+  if (Base.isVirtual())
+return false;
+

rnk wrote:
> OK, I guess vbases don't have a unique representation. :) What about vtable 
> slots? Should we check isDynamic?
Dynamic types are caught by "isTriviallyCopyable" pretty early in the process.  
I'll add a pair of tests to ensure that this remains the case.


https://reviews.llvm.org/D39064



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

[PATCH] D39064: implement __has_unique_object_representations

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 120132.
erichkeane added a comment.

Adding tests for excluding dynamic types.


https://reviews.llvm.org/D39064

Files:
  include/clang/AST/Type.h
  include/clang/Basic/TokenKinds.def
  include/clang/Basic/TypeTraits.h
  lib/AST/Type.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/type-traits.cpp

Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -716,6 +716,7 @@
 ///   '__is_sealed'   [MS]
 ///   '__is_trivial'
 ///   '__is_union'
+///   '__has_unique_object_representations'
 ///
 /// [Clang] unary-type-trait:
 ///   '__is_aggregate'
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2166,6 +2166,152 @@
   return false;
 }
 
+bool QualType::unionHasUniqueObjectRepresentations(
+const ASTContext &Context) const {
+  assert((*this)->isUnionType() && "must be union type");
+  CharUnits UnionSize = Context.getTypeSizeInChars(*this);
+  const RecordDecl *Union = getTypePtr()->getAs()->getDecl();
+
+  for (const auto *Field : Union->fields()) {
+if (!Field->getType().hasUniqueObjectRepresentations(Context))
+  return false;
+CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
+if (FieldSize != UnionSize)
+  return false;
+  }
+  return true;
+}
+
+bool isStructEmpty(QualType Ty) {
+  assert(Ty.getTypePtr()->isStructureOrClassType() &&
+ "Must be struct or class");
+  const RecordDecl *RD = Ty.getTypePtr()->getAs()->getDecl();
+
+  if (!RD->field_empty())
+return false;
+
+  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
+return ClassDecl->isEmpty();
+  }
+
+  return true;
+}
+
+bool QualType::structHasUniqueObjectRepresentations(
+const ASTContext &Context) const {
+  assert((*this)->isStructureOrClassType() && "Must be struct or class");
+  const RecordDecl *RD = getTypePtr()->getAs()->getDecl();
+
+  if (isStructEmpty(*this))
+return false;
+
+  // Check base types.
+  CharUnits BaseSize{};
+  if (const CXXRecordDecl *ClassDecl = dyn_cast(RD)) {
+for (const auto Base : ClassDecl->bases()) {
+  if (Base.isVirtual())
+return false;
+
+  // Empty bases are permitted, otherwise ensure base has unique
+  // representation. Also, Empty Base Optimization means that an
+  // Empty base takes up 0 size.
+  if (!isStructEmpty(Base.getType())) {
+if (!Base.getType().structHasUniqueObjectRepresentations(Context))
+  return false;
+BaseSize += Context.getTypeSizeInChars(Base.getType());
+  }
+}
+  }
+
+  CharUnits StructSize = Context.getTypeSizeInChars(*this);
+
+  // This struct obviously has bases that keep it from being 'empty', so
+  // checking fields is no longer required.  Ensure that the struct size
+  // is the sum of the bases.
+  if (RD->field_empty())
+return StructSize == BaseSize;
+  ;
+
+  CharUnits CurOffset =
+  Context.toCharUnitsFromBits(Context.getFieldOffset(*RD->field_begin()));
+
+  // If the first field isn't at the sum of the size of the bases, there
+  // is padding somewhere.
+  if (BaseSize != CurOffset)
+return false;
+
+  for (const auto *Field : RD->fields()) {
+if (!Field->getType().hasUniqueObjectRepresentations(Context))
+  return false;
+CharUnits FieldSize = Context.getTypeSizeInChars(Field->getType());
+CharUnits FieldOffset =
+Context.toCharUnitsFromBits(Context.getFieldOffset(Field));
+// Has padding between fields.
+if (FieldOffset != CurOffset)
+  return false;
+CurOffset += FieldSize;
+  }
+  // Check for tail padding.
+  return CurOffset == StructSize;
+}
+
+bool QualType::hasUniqueObjectRepresentations(const ASTContext &Context) const {
+  // C++17 [meta.unary.prop]:
+  //   The predicate condition for a template specialization
+  //   has_unique_object_representations shall be
+  //   satisfied if and only if:
+  // (9.1) — T is trivially copyable, and
+  // (9.2) — any two objects of type T with the same value have the same
+  // object representation, where two objects
+  //   of array or non-union class type are considered to have the same value
+  //   if their respective sequences of
+  //   direct subobjects have the same values, and two objects of union type
+  //   are considered to have the same
+  //   value if they have the same active member and the corresponding members
+  //   have the same value.
+  //   The set of scalar types for which this condition holds is
+  //   implementation-defined. [ Note: If a type has padding
+  //   bits, the condition does not hold; otherwise, the condition holds true
+  //   for unsigned integral types. — end
+  //   note ]
+  if (isNull())
+return false;
+
+  // Arrays are

[PATCH] D36952: [libclang] Add support for checking abstractness of records

[Johann Klähn via Phabricator via cfe-commits]

jklaehn added a comment.

ping :)


https://reviews.llvm.org/D36952



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

[PATCH] D35181: Defer addition of keywords to identifier table when loading AST

[Johann Klähn via Phabricator via cfe-commits]

jklaehn added a comment.

ping :)


https://reviews.llvm.org/D35181



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

r316500 - [Sema] Document+test the -Wsign-compare change for enums in C code [NFC]

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Tue Oct 24 14:05:43 2017
New Revision: 316500

URL: http://llvm.org/viewvc/llvm-project?rev=316500&view=rev
Log:
[Sema] Document+test the -Wsign-compare change for enums in C code [NFC]

rL316268 / D39122 has fixed PR35009, and now when in C,
these three(?) diagnostics properly use the enum's underlying
datatype.

While it was fixed, the test coverage was clearly insufficient,
because the -Wsign-compare change didn't show up in any of the
tests, until it was reported in the post-commit mail for rL316268.

So add the test for the -Wsign-compare diagnostic for enum
for C code, and while there, document this in the release notes.

The fix itself was obviously correct, so unless we want to silence
this new diagnosed case, i deem this commit to be NFC.

Added:
cfe/trunk/test/Sema/sign-compare-enum.c
Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=316500&r1=316499&r2=316500&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Oct 24 14:05:43 2017
@@ -82,6 +82,10 @@ Improvements to Clang's diagnostics
   tautological comparisons between integer variable of the type ``T`` and the
   largest/smallest possible integer constant of that same type.
 
+- For C code, ``-Wsign-compare``, ``-Wtautological-constant-compare`` and
+  ``-Wtautological-constant-out-of-range-compare`` were adjusted to use the
+  underlying datatype of ``enum``.
+
 - ``-Wnull-pointer-arithmetic`` now warns about performing pointer arithmetic
   on a null pointer. Such pointer arithmetic has an undefined behavior if the
   offset is nonzero. It also now warns about arithmetic on a null pointer

Added: cfe/trunk/test/Sema/sign-compare-enum.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/sign-compare-enum.c?rev=316500&view=auto
==
--- cfe/trunk/test/Sema/sign-compare-enum.c (added)
+++ cfe/trunk/test/Sema/sign-compare-enum.c Tue Oct 24 14:05:43 2017
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED 
-verify -Wsign-compare %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify 
-Wsign-compare %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED 
-DSILENCE -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -DSILENCE 
-verify %s
+
+int main() {
+  enum A { A_a = 0, A_b = 1 };
+  static const int message[] = {0, 1};
+  enum A a;
+
+  if (a < 2)
+return 0;
+
+#if defined(SIGNED) && !defined(SILENCE)
+  if (a < sizeof(message)/sizeof(message[0])) // expected-warning {{comparison 
of integers of different signs: 'enum A' and 'unsigned long long'}}
+return 0;
+#else
+  // expected-no-diagnostics
+  if (a < 2U)
+return 0;
+  if (a < sizeof(message)/sizeof(message[0]))
+return 0;
+#endif
+}


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