[PATCH] D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/Sema/nonnull.c:171
+
+void pr31040(char *p __attribute__((nonnull)));
+void pr31040(char *p) {}

Is this not pr30828?


https://reviews.llvm.org/D26800



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

[PATCH] D27099: [OpenCL] Prohibit using reserve_id_t in program scope.

[Egor Churaev via Phabricator via cfe-commits]

echuraev updated this revision to Diff 79508.

https://reviews.llvm.org/D27099

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/SemaOpenCL/event_t.cl
  test/SemaOpenCL/invalid-clk-events-cl2.0.cl
  test/SemaOpenCL/invalid-pipes-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipes-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipes-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipes-cl2.0.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 
+global pipe int gp;// expected-error {{type '__global read_only pipe int' can only be used as a function parameter in OpenCL}}
+global reserve_id_t rid;  // expected-error {{the '__global reserve_id_t' type cannot be used to declare a program scope variable}}
+
 void test1(pipe int *p) {// expected-error {{pipes packet types cannot be of reference type}}
 }
 void test2(pipe p) {// expected-error {{missing actual type specifier for pipe}}
@@ -20,3 +23,8 @@
 
 typedef pipe int pipe_int_t;
 pipe_int_t test6() {} // expected-error{{declaring function return value of type 'pipe_int_t' (aka 'read_only pipe int') is not allowed}}
+
+bool test_id_comprision(void) {
+  reserve_id_t id1, id2;
+  return (id1 == id2);  // expected-error {{invalid operands to binary expression ('reserve_id_t' and 'reserve_id_t')}}
+}
Index: test/SemaOpenCL/invalid-clk-events-cl2.0.cl
===
--- /dev/null
+++ test/SemaOpenCL/invalid-clk-events-cl2.0.cl
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+global clk_event_t ce; // expected-error {{the '__global clk_event_t' type cannot be used to declare a program scope variable}}
Index: test/SemaOpenCL/event_t.cl
===
--- test/SemaOpenCL/event_t.cl
+++ test/SemaOpenCL/event_t.cl
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 
-event_t glb_evt; // expected-error {{the event_t type cannot be used to declare a program scope variable}}
+event_t glb_evt; // expected-error {{the 'event_t' type cannot be used to declare a program scope variable}}
 
 constant struct evt_s {
   event_t evt; // expected-error {{the 'event_t' type cannot be used to declare a structure or union field}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -5909,32 +5909,31 @@
 return nullptr;
   }
 
-  // OpenCL v2.0 s6.9.b - Image type can only be used as a function argument.
-  // OpenCL v2.0 s6.13.16.1 - Pipe type can only be used as a function
-  // argument.
-  if (getLangOpts().OpenCL && (R->isImageType() || R->isPipeType())) {
-Diag(D.getIdentifierLoc(),
- diag::err_opencl_type_can_only_be_used_as_function_parameter)
-<< R;
-D.setInvalidType();
-return nullptr;
-  }
-
-  DeclSpec::SCS SCSpec = D.getDeclSpec().getStorageClassSpec();
-  StorageClass SC = StorageClassSpecToVarDeclStorageClass(D.getDeclSpec());
-
-  // dllimport globals without explicit storage class are treated as extern. We
-  // have to change the storage class this early to get the right DeclContext.
-  if (SC == SC_None && !DC->isRecord() &&
-  hasParsedAttr(S, D, AttributeList::AT_DLLImport) &&
-  !hasParsedAttr(S, D, AttributeList::AT_DLLExport))
-SC = SC_Extern;
+  if (getLangOpts().OpenCL) {
+// OpenCL v2.0 s6.9.b - Image type can only be used as a function argument.
+// OpenCL v2.0 s6.13.16.1 - Pipe type can only be used as a function
+// argument.
+if (R->isImageType() || R->isPipeType()) {
+  Diag(D.getIdentifierLoc(),
+   diag::err_opencl_type_can_only_be_used_as_function_parameter)
+  << R;
+  D.setInvalidType();
+  return nullptr;
+}
 
-  DeclContext *OriginalDC = DC;
-  bool IsLocalExternDecl = SC == SC_Extern &&
-   adjustContextForLocalExternDecl(DC);
+// OpenCL v1.2 s6.9.r:
+// The event type cannot be used to declare a program scope variable.
+// OpenCL v2.0 s6.9.q:
+// The clk_event_t and reserve_id_t types cannot be declared in program scope.
+if (NULL == S->getParent()) {
+  if (R->isReserveIDT() || R->isClkEventT() || R->isEventT()) {
+Diag(D.getIdentifierLoc(),
+ diag::err_invalid_type_for_program_scope_var) << R;
+D.setInvalidType();
+return nullptr;
+  }
+}
 
-  if (getLangOpts().OpenCL) {
 // OpenCL v1.0 s6.8.a.3: Pointers to functions are not allowed.
 QualType NR = R;
 while (NR->isPointerType()) {
@@ -5954,8 +5953,40 @@
 D.setInvalidType();
   }
 }
+
+// OpenCL v1.2 s6.9.b p4:
+// The sampler type cannot be used with the __local and __global address
+// space qualifiers.
+if (R->isSamplerT() && (R.getAddressSpace() == 

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

This seems like a great idea. btw, do you know about Cling 
(https://root.cern.ch/cling)?


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27187: [clang-tidy] Do not move parameter if only DeclRefExpr occurs inside of a loop

[Felix Berger via Phabricator via cfe-commits]

flx created this revision.
flx added reviewers: alexfh, sbenza, aaron.ballman.
flx added a subscriber: cfe-commits.
flx set the repository for this revision to rL LLVM.
flx added a project: clang-tools-extra.
Herald added a subscriber: JDevlieghere.

This fixes a bug where the performance-unnecessary-value-param check suggests a 
fix to move the parameter inside of a loop which could be invoked multiple 
times.


Repository:
  rL LLVM

https://reviews.llvm.org/D27187

Files:
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tidy/utils/DeclRefExprUtils.h
  test/clang-tidy/performance-unnecessary-value-param.cpp


Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -225,6 +225,15 @@
   // CHECK-FIXES: F = std::move(E);
 }
 
+// The argument could be moved but is not since copy statement is inside a 
loop.
+void PositiveNoMoveInsideLoop(ExpensiveMovableType E) {
+  // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the parameter 'E' is copied
+  // CHECK-FIXES: void PositiveNoMoveInsideLoop(const ExpensiveMovableType& E) 
{
+  for (;;) {
+auto F = E;
+  }
+}
+
 void PositiveConstRefNotMoveConstructible(ExpensiveToCopyType T) {
   // CHECK-MESSAGES: [[@LINE-1]]:63: warning: the parameter 'T' is copied
   // CHECK-FIXES: void PositiveConstRefNotMoveConstructible(const 
ExpensiveToCopyType& T) {
Index: clang-tidy/utils/DeclRefExprUtils.h
===
--- clang-tidy/utils/DeclRefExprUtils.h
+++ clang-tidy/utils/DeclRefExprUtils.h
@@ -47,6 +47,11 @@
 bool isCopyAssignmentArgument(const DeclRefExpr , const Stmt ,
   ASTContext );
 
+// Returns true if DeclRefExpr has a loop statement (ForStmt, CXXForRangeStmt,
+// WhileStmt, DoStmt) as ancestor.
+bool hasLoopStmtAncestor(const DeclRefExpr , const Stmt ,
+ ASTContext );
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/utils/DeclRefExprUtils.cpp
===
--- clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tidy/utils/DeclRefExprUtils.cpp
@@ -120,6 +120,17 @@
   return !Matches.empty();
 }
 
+bool hasLoopStmtAncestor(const DeclRefExpr , const Stmt ,
+ ASTContext ) {
+  auto Matches =
+  match(findAll(declRefExpr(equalsNode(),
+unless(hasAncestor(stmt(anyOf(
+forStmt(), cxxForRangeStmt(), whileStmt(),
+doStmt()).bind("declRef")),
+Stmt, Context);
+  return Matches.empty();
+}
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -104,6 +104,8 @@
   if (!IsConstQualified) {
 auto CanonicalType = Param->getType().getCanonicalType();
 if (AllDeclRefExprs.size() == 1 &&
+!utils::decl_ref_expr::hasLoopStmtAncestor(
+**AllDeclRefExprs.begin(), *Function->getBody(), *Result.Context) 
&&
 ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
   utils::decl_ref_expr::isCopyConstructorArgument(
   **AllDeclRefExprs.begin(), *Function->getBody(),


Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -225,6 +225,15 @@
   // CHECK-FIXES: F = std::move(E);
 }
 
+// The argument could be moved but is not since copy statement is inside a loop.
+void PositiveNoMoveInsideLoop(ExpensiveMovableType E) {
+  // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the parameter 'E' is copied
+  // CHECK-FIXES: void PositiveNoMoveInsideLoop(const ExpensiveMovableType& E) {
+  for (;;) {
+auto F = E;
+  }
+}
+
 void PositiveConstRefNotMoveConstructible(ExpensiveToCopyType T) {
   // CHECK-MESSAGES: [[@LINE-1]]:63: warning: the parameter 'T' is copied
   // CHECK-FIXES: void PositiveConstRefNotMoveConstructible(const ExpensiveToCopyType& T) {
Index: clang-tidy/utils/DeclRefExprUtils.h
===
--- clang-tidy/utils/DeclRefExprUtils.h
+++ clang-tidy/utils/DeclRefExprUtils.h
@@ -47,6 +47,11 @@
 bool isCopyAssignmentArgument(const DeclRefExpr , const Stmt ,
   ASTContext );
 
+// Returns true if DeclRefExpr has a loop statement (ForStmt, CXXForRangeStmt,
+// WhileStmt, DoStmt) as ancestor.

[PATCH] D27162: Support relaxed constexpr on chrono::duration operations

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a subscriber: cfe-commits.
AntonBikineev updated this revision to Diff 79491.

https://reviews.llvm.org/D27162

Files:
  include/chrono
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_++.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_++int.pass.cpp
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_+=.pass.cpp
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_--.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_--int.pass.cpp
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_-=.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_divide=.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
  www/cxx1z_status.html

Index: www/cxx1z_status.html
===
--- www/cxx1z_status.html
+++ www/cxx1z_status.html
@@ -132,7 +132,7 @@
 	http://wg21.link/P0502R0;>P0502R0LWGThrowing out of a parallel algorithm terminates - but how?Issaquah
 	http://wg21.link/P0503R0;>P0503R0LWGCorrecting library usage of "literal type"Issaquah
 	http://wg21.link/P0504R0;>P0504R0LWGRevisiting in-place tag types for any/optional/variantIssaquah
-	http://wg21.link/P0505R0;>P0505R0LWGWording for GB 50 - constexpr for chronoIssaquah
+	http://wg21.link/P0505R0;>P0505R0LWGWording for GB 50 - constexpr for chronoIssaquahComplete4.0
 	http://wg21.link/P0508R0;>P0508R0LWGWording for GB 58 - structured bindings for node_handlesIssaquah
 	http://wg21.link/P0509R1;>P0509R1LWGUpdating “Restrictions on exception handling”Issaquah
 	http://wg21.link/P0510R0;>P0510R0LWGDisallowing references, incomplete types, arrays, and empty variantsIssaquah
Index: test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
===
--- test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
+++ test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
@@ -11,14 +11,33 @@
 
 // duration
 
-// duration& operator*=(const rep& rhs);
+// constexpr duration& operator*=(const rep& rhs);
 
 #include 
 #include 
 
+#include "test_macros.h"
+
+#if TEST_STD_VER > 14
+constexpr std::chrono::hours test_ce(int n, int k) {
+std::chrono::hours h(n);
+h *= k;
+return h;
+}
+#endif
+
 int main()
 {
 std::chrono::nanoseconds ns(3);
 ns *= 5;
 assert(ns.count() == 15);
+
+#if TEST_STD_VER > 14
+{
+static_assert ( test_ce (7, 2) == std::chrono::hours(14), "" );
+static_assert ( test_ce (9, 5) == std::chrono::hours(45), "" );
+static_assert ( test_ce (5, 0) == std::chrono::hours(0), "" );
+static_assert ( test_ce (0, 0) == std::chrono::hours(0), "" );
+}
+#endif
 }
Index: test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
===
--- test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
+++ test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
@@ -11,14 +11,32 @@
 
 // duration
 
-// duration& operator%=(const rep& rhs)
+// constexpr duration& operator%=(const rep& rhs)
 
 #include 
 #include 
 
+#include "test_macros.h"
+
+#if TEST_STD_VER > 14
+constexpr std::chrono::hours test_ce(int n, int k) {
+std::chrono::hours h(n);
+h %= k;
+return h;
+}
+#endif
+
 int main()
 {
 std::chrono::microseconds us(11);
 us %= 3;
 assert(us.count() == 2);
+
+#if TEST_STD_VER > 14
+{
+static_assert ( test_ce (7, 2) == std::chrono::hours(1), "" );
+static_assert ( test_ce (9, 5) == std::chrono::hours(4), "" );
+static_assert ( test_ce (0, 4) == std::chrono::hours(0), "" );
+}
+#endif
 }
Index: test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
===
--- test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
+++ test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
@@ -11,17 +11,36 @@
 
 // duration
 
-// duration& operator%=(const duration& rhs)
+// constexpr duration& operator%=(const duration& rhs)
 
 #include 
 #include 
 
+#include "test_macros.h"
+
+#if TEST_STD_VER > 14
+constexpr std::chrono::hours test_ce(int n, int k) {
+typedef std::chrono::hours H;
+H h(n);
+h %= H(k);
+return h;
+}
+#endif
+
 int main()
 {
 std::chrono::microseconds us(11);
 std::chrono::microseconds us2(3);
 us %= us2;
 assert(us.count() == 2);
 us %= std::chrono::milliseconds(3);
 assert(us.count() == 2);
+

[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl

[Kareem Khazem via Phabricator via cfe-commits]

khazem created this revision.
khazem added reviewers: spyffe, a.sidorin.
khazem added subscribers: cfe-commits, phosek, seanklein, klimek.

Some of this patch comes from Aleksei's branch [1], with minor revisions. I've 
added unit tests and AST Matcher support. Copying in Manuel in case there is no 
need for a dedicated usingShadowDecl() matcher, in which case I could define it 
only in the test suite.

[1] 
https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp#L3594


https://reviews.llvm.org/D27181

Files:
  include/clang/AST/ASTImporter.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -558,5 +558,35 @@
 }
 
 
+TEST(ImportDecl, ImportUsingDecl) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+testImport(
+  "namespace foo { int bar; }"
+  "int declToImport(){ using foo::bar; }",
+  Lang_CXX, "", Lang_CXX, Verifier,
+  functionDecl(
+has(
+  compoundStmt(
+has(
+  declStmt(
+has(
+  usingDecl();
+}
+
+
+TEST(ImportDecl, ImportUsingShadowDecl) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+testImport(
+  "namespace foo { int bar; }"
+  "namespace declToImport { using foo::bar; }",
+  Lang_CXX, "", Lang_CXX, Verifier,
+  namespaceDecl(
+has(
+  usingShadowDecl();
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -420,6 +420,7 @@
   REGISTER_MATCHER(unresolvedUsingValueDecl);
   REGISTER_MATCHER(userDefinedLiteral);
   REGISTER_MATCHER(usingDecl);
+  REGISTER_MATCHER(usingShadowDecl);
   REGISTER_MATCHER(usingDirectiveDecl);
   REGISTER_MATCHER(valueDecl);
   REGISTER_MATCHER(varDecl);
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -175,6 +175,8 @@
 Decl *VisitObjCCategoryDecl(ObjCCategoryDecl *D);
 Decl *VisitObjCProtocolDecl(ObjCProtocolDecl *D);
 Decl *VisitLinkageSpecDecl(LinkageSpecDecl *D);
+Decl *VisitUsingDecl(UsingDecl *D);
+Decl *VisitUsingShadowDecl(UsingShadowDecl *D);
 
 ObjCTypeParamList *ImportObjCTypeParamList(ObjCTypeParamList *list);
 Decl *VisitObjCInterfaceDecl(ObjCInterfaceDecl *D);
@@ -4288,6 +4290,68 @@
   return ToLinkageSpec;
 }
 
+Decl *ASTNodeImporter::VisitUsingDecl(UsingDecl *D) {
+  DeclContext *DC, *LexicalDC;
+  DeclarationName Name;
+  SourceLocation Loc;
+  NamedDecl *AlreadyImported;
+  if (ImportDeclParts(D, DC, LexicalDC, Name, AlreadyImported, Loc))
+return NULL;
+  assert(DC && "Null DeclContext after importing decl parts");
+  if (AlreadyImported)
+return AlreadyImported;
+
+  DeclarationNameInfo NameInfo(Name, Importer.Import(D->getNameInfo().getLoc()));
+  ImportDeclarationNameLoc(D->getNameInfo(), NameInfo);
+
+  // If a UsingShadowDecl has a pointer to its UsingDecl, then we may
+  // already have imported this UsingDecl. That should have been caught
+  // above when checking if ToD is non-null.
+  assert(!Importer.GetImported(D) &&
+ "Decl imported but not assigned to AlreadyImported");
+
+  UsingDecl *ToD = UsingDecl::Create(Importer.getToContext(), DC,
+ Importer.Import(D->getUsingLoc()),
+ Importer.Import(D->getQualifierLoc()),
+ NameInfo, D->hasTypename());
+  ImportAttributes(D, ToD);
+  ToD->setLexicalDeclContext(LexicalDC);
+  LexicalDC->addDeclInternal(ToD);
+  Importer.Imported(D, ToD);
+
+  for (UsingShadowDecl *I : D->shadows()) {
+UsingShadowDecl *SD = cast(Importer.Import(I));
+ToD->addShadowDecl(SD);
+  }
+  return ToD;
+}
+
+Decl *ASTNodeImporter::VisitUsingShadowDecl(UsingShadowDecl *D) {
+  DeclContext *DC, *LexicalDC;
+  DeclarationName Name;
+  SourceLocation Loc;
+  NamedDecl *AlreadyImported;
+  if (ImportDeclParts(D, DC, LexicalDC, Name, AlreadyImported, Loc))
+return NULL;
+  assert(DC && "Null DeclContext after importing decl parts");
+  if (AlreadyImported)
+return AlreadyImported;
+
+  UsingShadowDecl *ToD = UsingShadowDecl::Create(
+Importer.getToContext(), DC, Loc,
+cast(Importer.Import(D->getUsingDecl())),
+cast_or_null(Importer.Import(D->getTargetDecl(;
+
+  ImportAttributes(D, ToD);
+  ToD->setAccess(D->getAccess());
+  ToD->setLexicalDeclContext(LexicalDC);
+  Importer.Imported(D, ToD);
+
+  

[PATCH] D26991: Hoist redundant load

[Marshall Clow via Phabricator via cfe-commits]

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

This looks fine to me - though I wonder if the compiler can hoist `*__first2` 
w/o us helping it.


https://reviews.llvm.org/D26991



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

Re: [PATCH] Warning for main returning a bool.

[Richard Smith via cfe-commits]

Committed as r288097.

On 28 November 2016 at 05:27, Joshua Hurwitz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Thanks Richard for looking at the revised patch.
>
> On Mon, Nov 21, 2016 at 1:50 PM Richard Smith 
> wrote:
>
>> This looks good to me. (While we could generalize this further, this
>> patch is a strict improvement, and we'll probably want to treat the 'main'
>> case specially regardless of whether we add a more general conversion
>> warning.)
>>
>> On 21 November 2016 at 07:18, Joshua Hurwitz  wrote:
>>
>> I modified the patch to warn only when a bool literal is returned from
>> main. See attached. -Josh
>>
>>
>> On Tue, Nov 15, 2016 at 7:50 PM Richard Smith 
>> wrote:
>>
>> On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel  wrote:
>> > - Original Message -
>> >> From: "Aaron Ballman" 
>> >> To: "Hal Finkel" 
>> >> Cc: "cfe-commits" , "Joshua Hurwitz" <
>> hurwi...@google.com>
>> >> Sent: Tuesday, November 15, 2016 4:42:05 PM
>> >> Subject: Re: [PATCH] Warning for main returning a bool.
>> >>
>> >> On Tue, Nov 15, 2016 at 5:22 PM, Hal Finkel  wrote:
>> >> > - Original Message -
>> >> >> From: "Aaron Ballman via cfe-commits" 
>> >> >> To: "Joshua Hurwitz" 
>> >> >> Cc: "cfe-commits" 
>> >> >> Sent: Tuesday, November 15, 2016 12:17:28 PM
>> >> >> Subject: Re: [PATCH] Warning for main returning a bool.
>> >> >>
>> >> >> On Fri, Oct 14, 2016 at 1:17 PM, Joshua Hurwitz via cfe-commits
>> >> >>  wrote:
>> >> >> > See attached.
>> >> >> >
>> >> >> > Returning a bool from main is a special case of return type
>> >> >> > mismatch. The
>> >> >> > common convention when returning a bool is that 'true' (== 1)
>> >> >> > indicates
>> >> >> > success and 'false' (== 0) failure. But since main expects a
>> >> >> > return
>> >> >> > value of
>> >> >> > 0 on success, returning a bool is usually unintended.
>> >> >>
>> >> >> I am not convinced that this is a high-value diagnostic. Returning
>> >> >> a
>> >> >> Boolean from main() may or may not be a bug (the returned value is
>> >> >> generally a convention more than anything else). Also, why Boolean
>> >> >> and
>> >> >> not, say, long long or float?
>> >> >
>> >> > I've seen this error often enough, but I think we need to be
>> >> > careful about false positives here. I recommend that we check only
>> >> > for explicit uses of boolean immediates (i.e. return true; or
>> >> > return false;) -- these are often bugs.
>> >>
>> >> I could get behind that.
>> >>
>> >> > Aaron, I disagree that the return value is just some kind of
>> >> > convention. It has a clear meaning.
>> >>
>> >> For many hosted environments, certainly. Freestanding
>> >> implementations?
>> >> Much less so, but I suppose this behavior is still reasonable enough
>> >> for them (not to mention, there may not even *be* a main() for a
>> >> freestanding implementation).
>> >>
>> >> > Furthermore, the behavior of the system can be quite different for
>> >> > a non-zero exit code than otherwise, and users who don't
>> >> > understand what's going on can find it very difficult to
>> >> > understand what's going wrong.
>> >>
>> >> That's a fair point, but my question still stands -- why only Boolean
>> >> values, and not "return 0x12345678ULL;" or "return 1.2;"?
>> >>
>> >> Combining with your idea above, if the check flagged instances where
>> >> a
>> >> literal of non-integral type (other than Boolean) is returned from
>> >> main(), that seems like good value.
>> >
>> > Agreed.
>> >
>> > FWIW, if we have a function that returns 'int' and the user tries to
>> return '1.2' we should probably warn for any function.
>>
>> Good point, we already have -Wliteral-conversion (which catches 1.2)
>> and -Wconstant-conversion (which catches 0x12345678ULL), and
>> -Wint-conversion for C programs returning awful things like string
>> literals, all of which are enabled by default. Perhaps Boolean values
>> really are the only case we don't explicitly warn about.
>>
>>
>> Wow, I'm amazed that we have no warning at all for converting, say,
>> 'true' to int (or indeed to float).
>>
>> Even with a warning for bool literal -> non-bool conversions in place, it
>> would still seem valuable to factor out a check for the corresponding case
>> in a return statement in main, since in that case we actually know what the
>> return value means, and the chance of a false positive goes way down.
>>
>> So I think that means we want two or possibly three checks here:
>>
>> 1) main returns bool literal
>> 2) -Wliteral-conversion warning for converting bool literal to another
>> type (excluding 1-bit unsigned integral 

r288097 - Add a warning for 'main' returning 'true' or 'false'.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Nov 28 19:35:17 2016
New Revision: 288097

URL: http://llvm.org/viewvc/llvm-project?rev=288097=rev
Log:
Add a warning for 'main' returning 'true' or 'false'.

Patch by Joshua Hurwitz!

Added:
cfe/trunk/test/Sema/warn-main-returns-bool-literal.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaStmt.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=288097=288096=288097=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Nov 28 19:35:17 
2016
@@ -646,6 +646,8 @@ def warn_main_one_arg : Warning<"only on
 def err_main_arg_wrong : Error<"%select{first|second|third|fourth}0 "
 "parameter of 'main' (%select{argument count|argument array|environment|"
 "platform-specific data}0) must be of type %1">;
+def warn_main_returns_bool_literal : Warning<"bool literal returned from "
+"'main'">, InGroup;
 def err_main_global_variable :
 Error<"main cannot be declared as global variable">;
 def warn_main_redefined : Warning<"variable named 'main' with external linkage 
"

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=288097=288096=288097=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Mon Nov 28 19:35:17 2016
@@ -3193,6 +3193,10 @@ StmtResult Sema::BuildReturnStmt(SourceL
 if (FD->isNoReturn())
   Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr)
 << FD->getDeclName();
+if (FD->isMain() && RetValExp)
+  if (isa(RetValExp))
+Diag(ReturnLoc, diag::warn_main_returns_bool_literal)
+  << RetValExp->getSourceRange();
   } else if (ObjCMethodDecl *MD = getCurMethodDecl()) {
 FnRetType = MD->getReturnType();
 isObjCMethod = true;

Added: cfe/trunk/test/Sema/warn-main-returns-bool-literal.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-main-returns-bool-literal.cpp?rev=288097=auto
==
--- cfe/trunk/test/Sema/warn-main-returns-bool-literal.cpp (added)
+++ cfe/trunk/test/Sema/warn-main-returns-bool-literal.cpp Mon Nov 28 19:35:17 
2016
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wmain -verify %s
+
+// expected-note@+1 {{previous definition is here}}
+int main() {
+  return 0;
+}  // no-warning
+
+// expected-error@+1 {{redefinition of 'main'}}
+int main() {
+  return 1.0;
+}  // no-warning
+
+int main() {
+  bool b = true;
+  return b;  // no-warning
+}
+
+int main() {
+  return true;  // expected-warning {{bool literal returned from 'main'}}
+}


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

[PATCH] D26950: [libc++abi] Add _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Ping.


https://reviews.llvm.org/D26950



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

[PATCH] D26949: [libc++abi] Clean up visibility

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Ping.

Lmk if there's any other tests I should run on this to ensure there's no 
functional difference.


https://reviews.llvm.org/D26949



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

[PATCH] D26657: [Sema] Respect DLL attributes more faithfully

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Ping.


https://reviews.llvm.org/D26657



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe created this revision.
spyffe added reviewers: a.sidorin, beanz, loladiro, v.g.vassilev.
spyffe added a subscriber: cfe-commits.
spyffe set the repository for this revision to rL LLVM.
Herald added a subscriber: mgorny.

LLVM's JIT is now the foundation of dynamic-compilation features for many 
languages.  Clang also has low-level support for dynamic compilation 
(`ASTImporter` and `ExternalASTSource`, notably).  How the compiler is set up 
for dynamic parsing is generally left up to individual clients, for example 
LLDB's C/C++/Objective-C expression parser and the ROOT project.

Although this arrangement offers external clients the flexibility to implement 
dynamic features as they see fit, the lack of an in-tree client means that 
subtle bugs can be introduced that cause regressions in the external clients 
but aren't caught by tests (or users) until much later.  LLDB for example 
regularly encounters complicated ODR violation scenarios where it is not 
immediately clear who is at fault.

I propose a simple expression parser be added to Clang.  I aim to have it 
encompass two main features:

- It should be able to look up external declarations from a variety of sources 
(e.g., from previous dynamic compilations, from modules, or from DWARF) and 
have clear conflict resolution rules with easily understood errors.  This 
functionality will be supported by in-tree tests.
- It should work hand in hand with the LLVM JIT to resolve the locations of 
external declarations so that e.g. variables can be redeclared and (for 
high-performance applications like DTrace) external variables can be accessed 
directly from the registers where they reside.

I have attached a tester that parses a sequence of source files and then uses 
them as source data for an expression.  External references are resolved using 
an `ExternalASTSource` that responds to name queries using an `ASTImporter`.  
This is the setup that LLDB uses, and the motivating reason for `MinimalImport` 
in `ASTImporter`.

Over time my intention is to make this more complete and support the many 
scenarios that LLDB can support.  I also want to identify places where LLDB 
does the wrong thing and we can make this functionality more correct, hopefully 
eliminating frustrating and opaque errors.  I also want to spot where Clang 
might get things wrong, and be able to point Clang developers to an 
easy-to-reproduce, in-tree test that doesn't require external projects or 
patches.  Finally, I want to identify how we can make this functionality as 
generic as possible for the current clients besides LLDB, and for potential 
future clients.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,347 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("-Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+static llvm::cl::opt LogLookups(
+"log-lookups",
+llvm::cl::desc("Print each lookup performed on behalf of the expression"));
+
+namespace {
+
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  

[PATCH] D27140: Allow clang to write compilation database records

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg updated this revision to Diff 79482.
joerg added a comment.

Move implementation into the Clang class, keep track of the raw_fd_stream. This 
avoids reopening it on multiple inputs and removes the need for append mode. 
Short circuit the function when -### is present. Add proper diagnostics on open 
failure.

Still not completely happy with the place and the associated need for the 
mutable member.


https://reviews.llvm.org/D27140

Files:
  docs/JSONCompilationDatabase.rst
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h

Index: lib/Driver/Tools.h
===
--- lib/Driver/Tools.h
+++ lib/Driver/Tools.h
@@ -17,6 +17,7 @@
 #include "clang/Driver/Util.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Option/Option.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Compiler.h"
 
 namespace clang {
@@ -101,6 +102,11 @@
 
   mutable std::unique_ptr CLFallback;
 
+  mutable std::unique_ptr CompilationDatabase = nullptr;
+  void DumpCompilationDatabase(StringRef Filename, StringRef Target,
+   const InputInfo , const InputInfo ,
+   const llvm::opt::ArgList ) const;
+
 public:
   // CAUTION! The first constructor argument ("clang") is not arbitrary,
   // as it is for other tools. Some operations on a Tool actually test
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -40,9 +40,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/Support/YAMLParser.h"
 
 #ifdef LLVM_ON_UNIX
 #include  // For getuid().
@@ -4002,6 +4002,60 @@
 CmdArgs.push_back("-KPIC");
 }
 
+void Clang::DumpCompilationDatabase(StringRef Filename, StringRef Target, const InputInfo ,
+const InputInfo , const ArgList ) const {
+
+  // If this is a dry run, do not create the compilation database file.
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+return;
+
+  using llvm::yaml::escape;
+  const Driver  = getToolChain().getDriver();
+
+  if (!CompilationDatabase) {
+std::error_code EC;
+auto File = llvm::make_unique(Filename, EC, llvm::sys::fs::F_Text);
+if (EC) {
+  D.Diag(clang::diag::err_drv_compilationdatabase) << Filename
+   << EC.message();
+  return;
+}
+CompilationDatabase = std::move(File);
+  }
+  auto  = *CompilationDatabase;
+  SmallString<128> Buf;
+  if (llvm::sys::fs::current_path(Buf))
+Buf = ".";
+  CDB << "{ \"directory\": \"" << escape(Buf) << "\"";
+  CDB << ", \"file\": \"" << escape(Input.getFilename()) << "\"";
+  CDB << ", \"output\": \"" << escape(Output.getFilename()) << "\"";
+  CDB << ", \"arguments\": [\"" << escape(D.ClangExecutable) << "\"";
+  Buf = "-x";
+  Buf += types::getTypeName(Input.getType());
+  CDB << ", \"" << escape(Buf) << "\"";
+  CDB << ", \"" << escape(Input.getFilename()) << "\"";
+  for (auto : Args) {
+auto  = A->getOption();
+// Skip language selection, which is positional.
+if (O.getID() == options::OPT_x)
+  continue;
+// Skip writing dependency output and the compilation database itself.
+if (O.getGroup().isValid() && O.getGroup().getID() == options::OPT_M_Group)
+  continue;
+// Skip inputs.
+if (O.getKind() == Option::InputClass)
+  continue;
+// All other arguments are quoted and appended.
+ArgStringList ASL;
+A->render(Args, ASL);
+for (auto : ASL)
+  CDB << ", \"" << escape(it) << "\"";
+  }
+  Buf = "--target=";
+  Buf += Target;
+  CDB << ", \"" << escape(Buf) << "\"]},\n";
+}
+
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const {
@@ -4046,6 +4100,11 @@
   CmdArgs.push_back("-triple");
   CmdArgs.push_back(Args.MakeArgString(TripleStr));
 
+  if (const Arg *MJ = Args.getLastArg(options::OPT_MJ)) {
+DumpCompilationDatabase(MJ->getValue(), TripleStr, Output, Input, Args);
+Args.ClaimAllArgs(options::OPT_MJ);
+  }
+
   if (IsCuda) {
 // We have to pass the triple of the host if compiling for a CUDA device and
 // vice-versa.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -266,6 +266,8 @@
 MetaVarName<"">;
 def MG : Flag<["-"], "MG">, Group, Flags<[CC1Option]>,
 HelpText<"Add missing headers to depfile">;
+def MJ : JoinedOrSeparate<["-"], "MJ">, Group,
+HelpText<"Write a compilation database entry per 

r288093 - Use ${:uid} to generate unique MS asm labels, not {:uid}

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Nov 28 18:39:37 2016
New Revision: 288093

URL: http://llvm.org/viewvc/llvm-project?rev=288093=rev
Log:
Use ${:uid} to generate unique MS asm labels, not {:uid}

Modified:
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm.cpp

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=288093=288092=288093=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Mon Nov 28 18:39:37 2016
@@ -753,7 +753,7 @@ LabelDecl *Sema::GetOrCreateMSAsmLabel(S
 // Create an internal name for the label.  The name should not be a valid 
mangled
 // name, and should be unique.  We use a dot to make the name an invalid 
mangled
 // name.
-OS << "__MSASMLABEL_.{:uid}__";
+OS << "__MSASMLABEL_.${:uid}__";
 for (char C : ExternalLabelName) {
   OS << C;
   // We escape '$' in asm strings by replacing it with "$$"

Modified: cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c?rev=288093=288092=288093=diff
==
--- cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c Mon Nov 28 18:39:37 2016
@@ -20,7 +20,7 @@ void invoke(void* that, unsigned methodI
 // CHECK: call void asm sideeffect inteldialect
 // CHECK: mov edx,dword ptr $1
 // CHECK: test edx,edx
-// CHECK: jz {{[^_]*}}__MSASMLABEL_.{:uid}__noparams
+// CHECK: jz {{[^_]*}}__MSASMLABEL_.${:uid}__noparams
 // ^ Can't use {{.*}} here because the matching is greedy.
 // CHECK: mov eax,edx
 // CHECK: shl eax,$$3
@@ -28,7 +28,7 @@ void invoke(void* that, unsigned methodI
 // CHECK: mov ecx,esp
 // CHECK: push dword ptr $0
 // CHECK: call dword ptr $2
-// CHECK: {{.*}}__MSASMLABEL_.{:uid}__noparams:
+// CHECK: {{.*}}__MSASMLABEL_.${:uid}__noparams:
 // CHECK: mov ecx,dword ptr $3
 // CHECK: push ecx
 // CHECK: mov edx,[ecx]

Modified: cfe/trunk/test/CodeGen/ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm.c?rev=288093=288092=288093=diff
==
--- cfe/trunk/test/CodeGen/ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c Mon Nov 28 18:39:37 2016
@@ -249,7 +249,7 @@ void t23() {
   the_label:
   }
 // CHECK: t23
-// CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.{:uid}__the_label:", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.${:uid}__the_label:", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t24_helper(void) {}
@@ -595,7 +595,7 @@ void label1() {
 jmp label
   }
   // CHECK-LABEL: define void @label1()
-  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.{:uid}__label:\0A\09jmp 
{{.*}}__MSASMLABEL_.{:uid}__label", "~{dirflag},~{fpsr},~{flags}"() 
[[ATTR1:#[0-9]+]]
+  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.${:uid}__label:\0A\09jmp 
{{.*}}__MSASMLABEL_.${:uid}__label", "~{dirflag},~{fpsr},~{flags}"() 
[[ATTR1:#[0-9]+]]
 }
 
 void label2() {
@@ -604,7 +604,7 @@ void label2() {
 label:
   }
   // CHECK-LABEL: define void @label2
-  // CHECK: call void asm sideeffect inteldialect "jmp 
{{.*}}__MSASMLABEL_.{:uid}__label\0A\09{{.*}}__MSASMLABEL_.{:uid}__label:", 
"~{dirflag},~{fpsr},~{flags}"()
+  // CHECK: call void asm sideeffect inteldialect "jmp 
{{.*}}__MSASMLABEL_.${:uid}__label\0A\09{{.*}}__MSASMLABEL_.${:uid}__label:", 
"~{dirflag},~{fpsr},~{flags}"()
 }
 
 void label3() {
@@ -613,7 +613,7 @@ void label3() {
 mov eax, label
   }
   // CHECK-LABEL: define void @label3
-  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.{:uid}__label:\0A\09mov eax, 
{{.*}}__MSASMLABEL_.{:uid}__label", "~{eax},~{dirflag},~{fpsr},~{flags}"()
+  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.${:uid}__label:\0A\09mov eax, 
{{.*}}__MSASMLABEL_.${:uid}__label", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void label4() {
@@ -622,7 +622,7 @@ void label4() {
 mov eax, [label]
   }
   // CHECK-LABEL: define void @label4
-  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.{:uid}__label:\0A\09mov eax, 
{{.*}}__MSASMLABEL_.{:uid}__label", "~{eax},~{dirflag},~{fpsr},~{flags}"()
+  // CHECK: call void asm sideeffect inteldialect 
"{{.*}}__MSASMLABEL_.${:uid}__label:\0A\09mov eax, 
{{.*}}__MSASMLABEL_.${:uid}__label", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void label5() {
@@ -631,7 +631,7 @@ void label5() {
 dollar_label$:
   }
   // CHECK-LABEL: define void @label5
-  // CHECK: call void 

[PATCH] D26839: [analyzer] An attempt to fix pr19539 - crashes on temporaries life-extended via members

[Devin Coughlin via Phabricator via cfe-commits]

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

This is great!




Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:205
 
-  // We need to be careful about treating a derived type's value as
-  // bindings for a base type. Unless we're creating a temporary pointer 
region,
-  // start by stripping and recording base casts.
-  SmallVector Casts;
-  const Expr *Inner = Ex->IgnoreParens();
-  if (!Loc::isLocType(Result->getType())) {
-while (const CastExpr *CE = dyn_cast(Inner)) {
-  if (CE->getCastKind() == CK_DerivedToBase ||
-  CE->getCastKind() == CK_UncheckedDerivedToBase)
-Casts.push_back(CE);
-  else if (CE->getCastKind() != CK_NoOp)
-break;
-
-  Inner = CE->getSubExpr()->IgnoreParens();
-}
-  }
+  // MaterializeTemporaryExpr may appear out of place, after a few field and
+  // base-class accesses have been made to the object, even though semantically

I think it would good to show an AST representation in the comment to make it 
clear what "out of place" means. This could be a very simplified version of the 
dumped AST for a temporary with a field access.


https://reviews.llvm.org/D26839



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

r288089 - Avoid lambdas in default member initializers to work around clang bug

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Nov 28 17:58:04 2016
New Revision: 288089

URL: http://llvm.org/viewvc/llvm-project?rev=288089=rev
Log:
Avoid lambdas in default member initializers to work around clang bug

On Windows, Clang is mangling lambdas in default member initializers
incorrectly. See PR31197.

This is causing redness on the self-host bots. Work around the problem
locally so we aren't blind to further issues.

Modified:
cfe/trunk/unittests/Tooling/LookupTest.cpp

Modified: cfe/trunk/unittests/Tooling/LookupTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/LookupTest.cpp?rev=288089=288088=288089=diff
==
--- cfe/trunk/unittests/Tooling/LookupTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp Mon Nov 28 17:58:04 2016
@@ -13,18 +13,19 @@ using namespace clang;
 
 namespace {
 struct GetDeclsVisitor : TestVisitor {
-  std::function OnCall = [&](CallExpr *Expr) {};
-  std::function OnRecordTypeLoc = [&](RecordTypeLoc Type) 
{
-  };
+  std::function OnCall;
+  std::function OnRecordTypeLoc;
   SmallVector DeclStack;
 
   bool VisitCallExpr(CallExpr *Expr) {
-OnCall(Expr);
+if (OnCall)
+  OnCall(Expr);
 return true;
   }
 
   bool VisitRecordTypeLoc(RecordTypeLoc Loc) {
-OnRecordTypeLoc(Loc);
+if (OnRecordTypeLoc)
+  OnRecordTypeLoc(Loc);
 return true;
   }
 


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

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Can I interprete that as LGTM otherwise?


Repository:
  rL LLVM

https://reviews.llvm.org/D27138



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

Re: [PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Richard Smith via cfe-commits]

On 28 November 2016 at 15:33, John McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Nov 28, 2016 at 2:48 PM, Richard Smith 
> wrote:
>
>> (Dropping Phabricator, since this isn't really about D27163...)
>>
>> On 28 November 2016 at 14:27, John McCall via Phabricator via cfe-commits
>>  wrote:
>>
>>> In https://reviews.llvm.org/D27163#607100, @rsmith wrote:
>>> > C has rather different and much less useful TBAA rules
>>>
>>> [...] I don't think the TBAA rules are as different as you're suggesting,
>>
>>
>> I can offhand think of at least the following differences, some of which
>> seem fairly major:
>>  * C does not allow changing the effective type of a declared variable,
>> C++ does.
>>
>
> Well, except that the name of the variable itself becomes formally unbound
> to an object, no?  But yes, you can change the effective type via an alias
> as long as you're careful to only use it through that alias, which is not
> permitted in C.
>
>
>>  * C does not support struct-path TBAA in any form (its model is that
>> lumps of memory have effective types, and it doesn't care how you got
>> there), C++ does.
>>
>
> You're assuming that a lump of memory can't have multiple effective types
> at once due to subobject overlap, which doesn't seem justified by the
> text.  In particular, a variable of struct type definitely has the
> effective type of that struct.
>

Sure, but effective types are only checked when the stored value is
accessed, which doesn't happen when naming a member of a struct or union,
only at the subsequent assignment or lvalue conversion, at which point the
lvalue expression has the type of the accessed subobject, so is presumably
valid per C11 6.5/7. (The lvalue expression won't have the type of some
enclosing struct here regardless of whether you used that struct type to
form an lvalue designating one of its members.)

If we really were supposed to imagine all objects enclosing the accessed
object for the purpose of C11 6.5/7, it's missing a rule allowing an lvalue
expression whose type is a member of the effective type of the object to
access the object. (It has the opposite rule, in order to allow accesses of
lvalues denoting entire structs to access the values stored in scalar
subobjects.)

 * C allows any assignment to a union member to change the effective type,
>> no matter whether it's locally determinable that you're storing to a union
>> member, C++ requires the assignment's LHS to be a union member access.
>>  * C only permits common initial sequence shenanigans for unions where
>> the union type is visible, C++ permits it anywhere.
>>
>
> The union rules in C are all over the place.
>

Agreed :)


>   I gave up trying to pretend that C has consistent union semantics a long
> time ago.
>
>
>> except that C++ actually tries to provide specific rules for unions (that
>>> don't match what users actually want).
>>
>>
>> In what way don't C++'s union rules match user expectations where C's
>> rules do? Do you mean type punning via unions? C doesn't allow that either
>> (or maybe it does, if you think a footnote carries more normative weight
>> than rules in the main text).
>>
>
> I was careful to say "what users actually want".  Users *want* type
> punning via unions. C's rules are contradictory because, as I read it, the
> committee has never been able to come to a consensus about how to formalize
> allowing type punning via unions without breaking the entire model — and to
> be fair, I don't know how to do that either.  But I would say that the
> expressed user desires are pretty clear here.
>

Interesting. What I see from users is a strong desire to be able to do
bitwise reinterpretation of values /somehow/, not necessarily to type-pun
through unions, which I think is subtly but importantly different. The C
DRs list has some implication that the C committee at some point intended
to make type-punning through unions work, but failed to actually make the
necessary changes in the wording (and all that we have left is a footnote).

Anyway, this is not actually important here, and I have not seen any code
> that falls into the cracks between language standards here, at least not
> under LLVM's intentionally-more-permissive-than-standardized application
> of TBAA.
>

I've seen plenty of programs that would be valid under some interpretations
of C's rules, but not under C++'s rules. But since we effectively provide
the C++ rules in all cases regardless, I agree this is not relevant for the
issue at hand.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I wonder whether it is possible to avoid calling 
DeclScopeObj.EnterDeclaratorScope at ParseDecl.cpp:5317 instead of fixing 
Sema::ActOnCXXEnterDeclaratorScope?

I believe it's calling EnterDeclaratorScope to enable name lookup when a 
namespace-member variable is defined outside the namespace. Since that's not 
the case here, it seems to me that it shouldn't called in the first place.

http://en.cppreference.com/w/cpp/language/unqualified_lookup


Repository:
  rL LLVM

https://reviews.llvm.org/D26922



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

Re: [PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[John McCall via cfe-commits]

On Mon, Nov 28, 2016 at 2:48 PM, Richard Smith 
wrote:

> (Dropping Phabricator, since this isn't really about D27163...)
>
> On 28 November 2016 at 14:27, John McCall via Phabricator via cfe-commits
>  wrote:
>
>> In https://reviews.llvm.org/D27163#607100, @rsmith wrote:
>> > C has rather different and much less useful TBAA rules
>>
>> [...] I don't think the TBAA rules are as different as you're suggesting,
>
>
> I can offhand think of at least the following differences, some of which
> seem fairly major:
>  * C does not allow changing the effective type of a declared variable,
> C++ does.
>

Well, except that the name of the variable itself becomes formally unbound
to an object, no?  But yes, you can change the effective type via an alias
as long as you're careful to only use it through that alias, which is not
permitted in C.


>  * C does not support struct-path TBAA in any form (its model is that
> lumps of memory have effective types, and it doesn't care how you got
> there), C++ does.
>

You're assuming that a lump of memory can't have multiple effective types
at once due to subobject overlap, which doesn't seem justified by the
text.  In particular, a variable of struct type definitely has the
effective type of that struct.


>  * C allows any assignment to a union member to change the effective type,
> no matter whether it's locally determinable that you're storing to a union
> member, C++ requires the assignment's LHS to be a union member access.
>  * C only permits common initial sequence shenanigans for unions where the
> union type is visible, C++ permits it anywhere.
>

The union rules in C are all over the place.  I gave up trying to pretend
that C has consistent union semantics a long time ago.


> except that C++ actually tries to provide specific rules for unions (that
>> don't match what users actually want).
>
>
> In what way don't C++'s union rules match user expectations where C's
> rules do? Do you mean type punning via unions? C doesn't allow that either
> (or maybe it does, if you think a footnote carries more normative weight
> than rules in the main text).
>

I was careful to say "what users actually want".  Users *want* type punning
via unions. C's rules are contradictory because, as I read it, the
committee has never been able to come to a consensus about how to formalize
allowing type punning via unions without breaking the entire model — and to
be fair, I don't know how to do that either.  But I would say that the
expressed user desires are pretty clear here.

Anyway, this is not actually important here, and I have not seen any code
that falls into the cracks between language standards here, at least not
under LLVM's intentionally-more-permissive-than-standardized application of
TBAA.

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

[PATCH] Update usage of RHEL devtoolsets

[Michael Lampe via cfe-commits]

- remove devtoolset-1 (obsoleted gcc 4.7)
- add devtoolset-6 (gcc 6.2+)

(There is no devtoolset-5, RH switched to gcc major version numbering.)

Please apply, I don't have commit access.

Thanks,

Michael


diff -ru a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
--- a/lib/Driver/ToolChains.cpp 2016-11-26 03:21:16.353564220 +0100
+++ b/lib/Driver/ToolChains.cpp 2016-11-26 03:24:40.096334795 +0100
@@ -1441,11 +1441,10 @@
 // Then look for distribution supplied gcc installations.
 if (D.SysRoot.empty()) {
   // Look for RHEL devtoolsets.
+  Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
   Prefixes.push_back("/opt/rh/devtoolset-4/root/usr");
   Prefixes.push_back("/opt/rh/devtoolset-3/root/usr");
   Prefixes.push_back("/opt/rh/devtoolset-2/root/usr");
-  Prefixes.push_back("/opt/rh/devtoolset-1.1/root/usr");
-  Prefixes.push_back("/opt/rh/devtoolset-1.0/root/usr");
   // And finally in /usr.
   Prefixes.push_back("/usr");
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

[Łukasz Anforowicz via Phabricator via cfe-commits]

lukasza added a comment.

Forcing shallow matching means that unit test below will stop passing after 
this CL.

  TEST(HasDeclaration, DeepTagType) {
std::string input =
"class Foo {};\n"
"using Bar = Foo;\n"
"void Function(Bar param) {}\n";
  
// Matcher for declaration of the Foo class.
auto param_type_decl_matcher = cxxRecordDecl(hasName("Foo"));
  
// hasDeclaration / qualType-flavour.
EXPECT_TRUE(matches(input, parmVarDecl(
hasName("param"),
hasType(qualType(hasDeclaration(decl(param_type_decl_matcher)));
  }

I am working on a tool that renames methods in a specific namespace - the tool 
depends on hasDeclaration doing deep-matching to tell whether an 
UnresolvedUsingValueDecl and/or CXXDependentScopeMemberExpr end up pointing at 
a record or template from the namespace of interest.

Q1: I assume that the breaking aspect of this CL is understood and accepted?

Q2: Can you please provide an example code (here or in the CL description) for 
how to achieve deep matching (since AFAIU hasDeclaration would no longer do 
deep matching after the CL under review)?  Can deep matching be achieved by 
combining existing matchers with the now shallow hasDeclaration?  Or does deep 
matching require authoring a custom matcher?  How would the custom matcher look 
like (i.e. would it have to consider all possible types that can be desugared - 
e.g. requiring separate code for each possible type node - i.e. having separate 
code for hopping over a type alias and over other type nodes).




Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:250
 }
 
 TEST(HasDeclaration, HasDeclarationOfCXXNewExpr) {

Could you please add a unit test that covers a scenario made possible by your 
CL and which involves an elaborated type?

TEST(HasDeclaration, ElaboratedTypeAndTemplateSpecializationType) {
  std::string input =
  "namespace Namespace {\n"
  "template\n"
  "class Template {\n"
  " public:\n"
  "  void Method() {}\n"
  "};\n"
  "}  // namespace Namespace\n"
  "template \n"
  "void Function(Namespace::Template param) {\n"
  "  param.Method();\n"
  "};\n";

  // Matcher for ::Namespace::Template template decl.
  auto param_type_decl_matcher = classTemplateDecl(
  hasName("Template"),
  hasParent(namespaceDecl(hasName("Namespace"))),
  has(templateTypeParmDecl(hasName("T";

  // hasDeclaration / qualType-flavour.
  EXPECT_TRUE(matches(input, parmVarDecl(
  hasName("param"),
  hasType(qualType(hasDeclaration(decl(param_type_decl_matcher)));
}

I was a little bit surprised that ElaboratedType is handled when going via 
qualType, but not when matching the type directly - the test code below (an 
extension of the unit test proposed above) fails to compile.  Is this expected 
(for this CL?  for long-term?)?

  // hasDeclaration / elaboratedType-flavour.
  EXPECT_TRUE(matches(input, parmVarDecl(
  hasName("param"),
  hasType(elaboratedType(hasDeclaration(decl(param_type_decl_matcher)));



https://reviews.llvm.org/D27104



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

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Great, I'll get this working soon and attach a new patch :)

On Mon, 28 Nov 2016 at 14:27 Hans Wennborg  wrote:

> On Mon, Nov 28, 2016 at 11:11 AM, Antonio Maiorano 
> wrote:
> >> It's built with the script in utils/release/build_llvm_package.bat
> > which I run manually on my machine once every few weeks.
> >
> > Okay, that's good news. So the simplest path to success would be to
> require
> > the user to either pass the path to CMake via an arg like
> -DNUGET_EXE_PATH,
> > or if it's not defined, to assume it's already in PATH. This is the most
> > future-proof solution as it will work with future versions of VS (2017 RC
> > just came out).
> >
> > I can still look into whether a vsix built with VS 2015 references will
> > continue to work in older versions of VS, but even if this works, I feel
> > like it's a temporary solution at best. There are other advantages to
> using
> > NuGet here: it would allow us to more easily pin/upgrade which
> assemblies we
> > want to use over time.
> >
> > If you're okay with it, I'll make the changes necessary to use
> > -DNUGET_EXE_PATH, if defined, otherwise assume it's on PATH. This should
> be
> > a simple change at this point.
>
> That sounds good to me. There are already a bunch of prerequisites for
> building the plugin, so adding this one doesn't seem unreasonable.
> Especially since it seems it will simplify things to the point that
> they might even work elsewhere than my own machine :-)
>
>
> > On Mon, 28 Nov 2016 at 13:59 Hans Wennborg  wrote:
> >>
> >> On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano  >
> >> wrote:
> >> > Okay, I'll see if upgrading to the 2015 assemblies would allow the
> VSIX
> >> > to
> >> > keep working in older versions of VS.
> >> >
> >> > Still waiting on an answer to this question:
> >> >
> >> >> In either case, though, I must ask: how is the offical vsix that's
> >> >> available on http://llvm.org/builds/ get built? Is it part of an
> >> >> automated
> >> >> Clang build, or is it built and uploaded manually? If it's automated,
> >> >> then
> >> >> having to download and point to nuget.exe won't work.
> >>
> >> It's built with the script in utils/release/build_llvm_package.bat
> >> which I run manually on my machine once every few weeks.
> >>
> >>
> >> > On Mon, 28 Nov 2016 at 13:04 Hans Wennborg  wrote:
> >> >>
> >> >> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano <
> amaior...@gmail.com>
> >> >> wrote:
> >> >> > Ah, no, that's not what I meant. The required referenced assemblies
> >> >> > are
> >> >> > versions that are normally installed with VS 2010.
> >> >> >
> >> >> > The first time I worked on this, I had upgraded the referenced
> >> >> > assemblies to
> >> >> > the ones that ship with VS 2015, but then there was question of
> >> >> > whether
> >> >> > or
> >> >> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not
> >> >> > sure
> >> >> > if
> >> >> > it would work, but I guess I can try to figure that out.
> >> >>
> >> >> Let me know if you figure this one out. It sounds like it would
> >> >> simplify things a lot.
> >> >>
> >> >> > In any case, what I discovered is that the "right" way to do things
> >> >> > to
> >> >> > make
> >> >> > sure your extension compiles in future versions of VS is to use
> NuGet
> >> >> > to
> >> >> > automatically pull in the required assemblies, or to check them in
> >> >> > and
> >> >> > reference them directly. The former would be better except for the
> >> >> > problem
> >> >> > of CLI builds as I described in my earlier email.
> >> >> >
> >> >> >
> >> >> >
> >> >> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner 
> >> >> > wrote:
> >> >> >>
> >> >> >> Sorry, i think I misunderstood the original option 1. I
> interpreted
> >> >> >> it
> >> >> >> as
> >> >> >> just committing changes to the vsix manifest to reference a
> specific
> >> >> >> version
> >> >> >> of the assembly which we assume to be present since it should be
> >> >> >> automatically installed with vs 2015. Is this not possible? Can't
> we
> >> >> >> just
> >> >> >> point the manifest to the version installed with vs?
> >> >> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano
> >> >> >> 
> >> >> >> wrote:
> >> >> >>>
> >> >> >>> Hi again,
> >> >> >>>
> >> >> >>> I've made the changes so that the required assemblies are
> >> >> >>> committed,
> >> >> >>> so
> >> >> >>> now we can build the clang-format-vsix with just VS 2015. Since
> the
> >> >> >>> patch
> >> >> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if
> >> >> >>> you'd
> >> >> >>> rather I attach it, let me know):
> >> >> >>>
> >> >> >>>
> >> >> >>>
> >> >> >>>
> >> >> >>>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
> >> >> >>>
> >> >> >>> Note that it's a git patch set, for which I followed the
> >> >> >>> 

[PATCH] D26082: Support for Python 3 in libclang python bindings

[Mathieu Duponchelle via cfe-commits]

MathieuDuponchelle added a comment.

I get tons of errors when running these tests with python 3 (version 3.4.3) 
with a fresh build of clang and llvm. The number of failures vs errors vary 
wildly between each invocation, the number of total failures + errors stays the 
same, which led me to think there was some sort of memory corruption.

Running the tests under valgrind make them pass, but also shows errors which I 
assume are ctypes-related.

This is valgrind's output for ` 
LD_LIBRARY_PATH=/home/meh/devel/sandbox/llvm/build/lib valgrind python3 -m nose 
tests.cindex.test_cursor:test_get_template_argument_value` as an example

F2616611: valgrind_output.txt 


Repository:
  rL LLVM

https://reviews.llvm.org/D26082



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

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX to
keep working in older versions of VS.

Still waiting on an answer to this question:

> In either case, though, I must ask: how is the offical vsix that's
available on http://llvm.org/builds/ get built? Is it part of an automated
Clang build, or is it built and uploaded manually? If it's automated, then
having to download and point to nuget.exe won't work.

Thanks!

On Mon, 28 Nov 2016 at 13:04 Hans Wennborg  wrote:

> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano 
> wrote:
> > Ah, no, that's not what I meant. The required referenced assemblies are
> > versions that are normally installed with VS 2010.
> >
> > The first time I worked on this, I had upgraded the referenced
> assemblies to
> > the ones that ship with VS 2015, but then there was question of whether
> or
> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure
> if
> > it would work, but I guess I can try to figure that out.
>
> Let me know if you figure this one out. It sounds like it would
> simplify things a lot.
>
> > In any case, what I discovered is that the "right" way to do things to
> make
> > sure your extension compiles in future versions of VS is to use NuGet to
> > automatically pull in the required assemblies, or to check them in and
> > reference them directly. The former would be better except for the
> problem
> > of CLI builds as I described in my earlier email.
> >
> >
> >
> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner  wrote:
> >>
> >> Sorry, i think I misunderstood the original option 1. I interpreted it
> as
> >> just committing changes to the vsix manifest to reference a specific
> version
> >> of the assembly which we assume to be present since it should be
> >> automatically installed with vs 2015. Is this not possible? Can't we
> just
> >> point the manifest to the version installed with vs?
> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
> >> wrote:
> >>>
> >>> Hi again,
> >>>
> >>> I've made the changes so that the required assemblies are committed, so
> >>> now we can build the clang-format-vsix with just VS 2015. Since the
> patch
> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
> >>> rather I attach it, let me know):
> >>>
> >>>
> >>>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
> >>>
> >>> Note that it's a git patch set, for which I followed the instructions
> >>> here.
> >>>
> >>> Thanks.
> >>>
> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano 
> >>> wrote:
> 
>  Okay, that's fine, I'll go for that and if all looks good, will
> attach a
>  patch.
> 
>  Thanks.
> 
>  On Thu, 24 Nov 2016 at 15:09 Zachary Turner 
> wrote:
> >
> > I would use the first solution. We lock ourselves to specific
> versions
> > of vs, so i think it's fine to do the same with the assemblies and
> deal with
> > it only when we upgrade
> > On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano <
> amaior...@gmail.com>
> > wrote:
> >>
> >> Hi Hans,
> >>
> >> I saw that on September 15th, you checked in a change: clang-format
> VS
> >> plugin: upgrade the project files to VS2015.
> >>
> >> When I open the latest version of ClangFormat.sln on a machine that
> >> has only VS 2015, it doesn't build. The reason is that some of the
> >> referenced assemblies are from VS 2010.
> >>
> >>   >> Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0,
> Culture=neutral,
> >> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
> >>
> >> What happens is that when building, these specific assemblies are
> not
> >> found. I suspect you have VS 2010 installed on your machine, which
> is why
> >> you don't get these build errors.
> >>
> >> The recommended way to handle this is to make use of NuGet to have
> it
> >> automatically download the required assemblies. I have made the
> changes
> >> locally to get this working, and it works great when building
> >> ClangFormat.sln from within Visual Studio; however, building from
> the CLI
> >> doesn't work out of the box because you must explicitly run
> 'nuget.exe
> >> restore ClangFormat.sln' before running msbuild (or devenv.exe in
> our case).
> >> The problem is that nuget.exe isn't something that's easily
> found/accessible
> >> on Windows, even once installed as an extension in VS. This is a
> known
> >> problem and the current best solution requires downloading and
> making
> >> nuget.exe available in PATH.
> >>
> >> So now i'm faced with figuring out how best to solve this so that
> the
> >> custom build step in this CMakeLists.txt that runs devenv doesn't
> fail:
> >>
> >> devenv 

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

> It's built with the script in utils/release/build_llvm_package.bat
which I run manually on my machine once every few weeks.

Okay, that's good news. So the simplest path to success would be to require
the user to either pass the path to CMake via an arg like -DNUGET_EXE_PATH,
or if it's not defined, to assume it's already in PATH. This is the most
future-proof solution as it will work with future versions of VS (2017 RC
just came out).

I can still look into whether a vsix built with VS 2015 references will
continue to work in older versions of VS, but even if this works, I feel
like it's a temporary solution at best. There are other advantages to using
NuGet here: it would allow us to more easily pin/upgrade which assemblies
we want to use over time.

If you're okay with it, I'll make the changes necessary to use
-DNUGET_EXE_PATH, if defined, otherwise assume it's on PATH. This should be
a simple change at this point.


On Mon, 28 Nov 2016 at 13:59 Hans Wennborg  wrote:

> On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano 
> wrote:
> > Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX
> to
> > keep working in older versions of VS.
> >
> > Still waiting on an answer to this question:
> >
> >> In either case, though, I must ask: how is the offical vsix that's
> >> available on http://llvm.org/builds/ get built? Is it part of an
> automated
> >> Clang build, or is it built and uploaded manually? If it's automated,
> then
> >> having to download and point to nuget.exe won't work.
>
> It's built with the script in utils/release/build_llvm_package.bat
> which I run manually on my machine once every few weeks.
>
>
> > On Mon, 28 Nov 2016 at 13:04 Hans Wennborg  wrote:
> >>
> >> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano 
> >> wrote:
> >> > Ah, no, that's not what I meant. The required referenced assemblies
> are
> >> > versions that are normally installed with VS 2010.
> >> >
> >> > The first time I worked on this, I had upgraded the referenced
> >> > assemblies to
> >> > the ones that ship with VS 2015, but then there was question of
> whether
> >> > or
> >> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not
> sure
> >> > if
> >> > it would work, but I guess I can try to figure that out.
> >>
> >> Let me know if you figure this one out. It sounds like it would
> >> simplify things a lot.
> >>
> >> > In any case, what I discovered is that the "right" way to do things to
> >> > make
> >> > sure your extension compiles in future versions of VS is to use NuGet
> to
> >> > automatically pull in the required assemblies, or to check them in and
> >> > reference them directly. The former would be better except for the
> >> > problem
> >> > of CLI builds as I described in my earlier email.
> >> >
> >> >
> >> >
> >> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner 
> wrote:
> >> >>
> >> >> Sorry, i think I misunderstood the original option 1. I interpreted
> it
> >> >> as
> >> >> just committing changes to the vsix manifest to reference a specific
> >> >> version
> >> >> of the assembly which we assume to be present since it should be
> >> >> automatically installed with vs 2015. Is this not possible? Can't we
> >> >> just
> >> >> point the manifest to the version installed with vs?
> >> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano <
> amaior...@gmail.com>
> >> >> wrote:
> >> >>>
> >> >>> Hi again,
> >> >>>
> >> >>> I've made the changes so that the required assemblies are committed,
> >> >>> so
> >> >>> now we can build the clang-format-vsix with just VS 2015. Since the
> >> >>> patch
> >> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if
> you'd
> >> >>> rather I attach it, let me know):
> >> >>>
> >> >>>
> >> >>>
> >> >>>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
> >> >>>
> >> >>> Note that it's a git patch set, for which I followed the
> instructions
> >> >>> here.
> >> >>>
> >> >>> Thanks.
> >> >>>
> >> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano 
> >> >>> wrote:
> >> 
> >>  Okay, that's fine, I'll go for that and if all looks good, will
> >>  attach a
> >>  patch.
> >> 
> >>  Thanks.
> >> 
> >>  On Thu, 24 Nov 2016 at 15:09 Zachary Turner 
> >>  wrote:
> >> >
> >> > I would use the first solution. We lock ourselves to specific
> >> > versions
> >> > of vs, so i think it's fine to do the same with the assemblies and
> >> > deal with
> >> > it only when we upgrade
> >> > On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano
> >> > 
> >> > wrote:
> >> >>
> >> >> Hi Hans,
> >> >>
> >> >> I saw that on September 15th, you checked in a change:
> clang-format
> >> >> VS
> >> >> plugin: upgrade the project files 

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Unfortunately, vsvarsall doesn't bring nuget onto path. I shared a few
links earlier about this:
https://nuget.codeplex.com/workitem/3615
http://stackoverflow.com/questions/22300375/nuget-auto-package-restore-does-not-work-with-msbuild/23935892#23935892

Your idea about -DMSVC_NUGET_PATH for CMake is interesting, although at
that point, we may as well just ask the user to download
https://nuget.org/nuget.exe and add it to their PATH. In either case,
though, I must ask: how is the offical vsix that's available on
http://llvm.org/builds/ get built? Is it part of an automated Clang build,
or is it built and uploaded manually? If it's automated, then having to
download and point to nuget.exe won't work.

On Fri, 25 Nov 2016 at 22:15 Zachary Turner  wrote:

> Does running vcvarsall put nuget on the path?
>
> What if we require the user to specify the path to nuget in some CMake
> variable? -DMSVC_NUGET_PATH=foo?
> On Fri, Nov 25, 2016 at 6:58 PM Antonio Maiorano 
> wrote:
>
> Ah, no, that's not what I meant. The required referenced assemblies are
> versions that are normally installed with VS 2010.
>
> The first time I worked on this, I had upgraded the referenced assemblies
> to the ones that ship with VS 2015, but then there was question of whether
> or not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure
> if it would work, but I guess I can try to figure that out.
>
> In any case, what I discovered is that the "right" way to do things to
> make sure your extension compiles in future versions of VS is to use NuGet
> to automatically pull in the required assemblies, or to check them in and
> reference them directly. The former would be better except for the problem
> of CLI builds as I described in my earlier email.
>
>
>
> On Fri, 25 Nov 2016 at 21:47 Zachary Turner  wrote:
>
> Sorry, i think I misunderstood the original option 1. I interpreted it as
> just committing changes to the vsix manifest to reference a specific
> version of the assembly which we assume to be present since it should be
> automatically installed with vs 2015. Is this not possible? Can't we just
> point the manifest to the version installed with vs?
> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
> wrote:
>
> Hi again,
>
> I've made the changes so that the required assemblies are committed, so
> now we can build the clang-format-vsix with just VS 2015. Since the patch
> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
> rather I attach it, let me know):
>
>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>
> Note that it's a git patch set, for which I followed the instructions here
> .
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano  wrote:
>
> Okay, that's fine, I'll go for that and if all looks good, will attach a
> patch.
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:
>
> I would use the first solution. We lock ourselves to specific versions of
> vs, so i think it's fine to do the same with the assemblies and deal with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>
> Hi Hans,
>
> I saw that on September 15th, you checked in a change: clang-format VS
> plugin: upgrade the project files to VS2015.
>
> When I open the latest version of ClangFormat.sln on a machine that has
> only VS 2015, it doesn't build. The reason is that some of the referenced
> assemblies are from VS 2010.
>
>   Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, Culture=neutral,
> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>
> What happens is that when building, these specific assemblies are not
> found. I suspect you have VS 2010 installed on your machine, which is why
> you don't get these build errors.
>
> The recommended way to handle this
> 
> is to make use of NuGet to have it automatically download the required
> assemblies. I have made the changes locally to get this working, and it
> works great when building ClangFormat.sln from within Visual Studio;
> however, building from the CLI doesn't work out of the box because you must
> explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
> (or devenv.exe in our case). The problem is that nuget.exe isn't something
> that's easily found/accessible on Windows, even once installed as an
> extension in VS. This is a known problem
>  and the current best solution
> requires downloading and making nuget.exe available in PATH
> .
>
> So now i'm faced with figuring out how best to solve this so that the

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Ah, no, that's not what I meant. The required referenced assemblies are
versions that are normally installed with VS 2010.

The first time I worked on this, I had upgraded the referenced assemblies
to the ones that ship with VS 2015, but then there was question of whether
or not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure
if it would work, but I guess I can try to figure that out.

In any case, what I discovered is that the "right" way to do things to make
sure your extension compiles in future versions of VS is to use NuGet to
automatically pull in the required assemblies, or to check them in and
reference them directly. The former would be better except for the problem
of CLI builds as I described in my earlier email.



On Fri, 25 Nov 2016 at 21:47 Zachary Turner  wrote:

> Sorry, i think I misunderstood the original option 1. I interpreted it as
> just committing changes to the vsix manifest to reference a specific
> version of the assembly which we assume to be present since it should be
> automatically installed with vs 2015. Is this not possible? Can't we just
> point the manifest to the version installed with vs?
> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
> wrote:
>
> Hi again,
>
> I've made the changes so that the required assemblies are committed, so
> now we can build the clang-format-vsix with just VS 2015. Since the patch
> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
> rather I attach it, let me know):
>
>
> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>
> Note that it's a git patch set, for which I followed the instructions here
> .
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano  wrote:
>
> Okay, that's fine, I'll go for that and if all looks good, will attach a
> patch.
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:
>
> I would use the first solution. We lock ourselves to specific versions of
> vs, so i think it's fine to do the same with the assemblies and deal with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>
> Hi Hans,
>
> I saw that on September 15th, you checked in a change: clang-format VS
> plugin: upgrade the project files to VS2015.
>
> When I open the latest version of ClangFormat.sln on a machine that has
> only VS 2015, it doesn't build. The reason is that some of the referenced
> assemblies are from VS 2010.
>
>   Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, Culture=neutral,
> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>
> What happens is that when building, these specific assemblies are not
> found. I suspect you have VS 2010 installed on your machine, which is why
> you don't get these build errors.
>
> The recommended way to handle this
> 
> is to make use of NuGet to have it automatically download the required
> assemblies. I have made the changes locally to get this working, and it
> works great when building ClangFormat.sln from within Visual Studio;
> however, building from the CLI doesn't work out of the box because you must
> explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
> (or devenv.exe in our case). The problem is that nuget.exe isn't something
> that's easily found/accessible on Windows, even once installed as an
> extension in VS. This is a known problem
>  and the current best solution
> requires downloading and making nuget.exe available in PATH
> .
>
> So now i'm faced with figuring out how best to solve this so that the
> custom build step in this CMakeLists.txt
> 
>  that
> runs devenv doesn't fail:
>
> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release
>
> There are a few options:
>
> 1) Forget NuGet and just commit the referenced assemblies. This is the
> simplest solution, but is more annoying to manage if we want to upgrade the
> versions of these assemblies later.
>
> 2) Commit nuget.exe to the repo and just use it. This is simple enough,
> but I'm not sure how people feel about committing binaries, and it would be
> frozen at whatever version we commit.
>
> 3) Do some form of wget to grab the latest nuget.exe from "
> https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
> what's the simplest way to do this. Powershell could be used, but there are
> security annoyances to deal with.
>
> That's all I can come up with so far. Would love to hear from you guys if
> you have any ideas or opinions on this. If you want I can send you a 

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Hi Hans,

I saw that on September 15th, you checked in a change: clang-format VS
plugin: upgrade the project files to VS2015.

When I open the latest version of ClangFormat.sln on a machine that has
only VS 2015, it doesn't build. The reason is that some of the referenced
assemblies are from VS 2010.

 

What happens is that when building, these specific assemblies are not
found. I suspect you have VS 2010 installed on your machine, which is why
you don't get these build errors.

The recommended way to handle this

is to make use of NuGet to have it automatically download the required
assemblies. I have made the changes locally to get this working, and it
works great when building ClangFormat.sln from within Visual Studio;
however, building from the CLI doesn't work out of the box because you must
explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
(or devenv.exe in our case). The problem is that nuget.exe isn't something
that's easily found/accessible on Windows, even once installed as an
extension in VS. This is a known problem
 and the current best solution
requires downloading and making nuget.exe available in PATH
.

So now i'm faced with figuring out how best to solve this so that the
custom build step in this CMakeLists.txt

that
runs devenv doesn't fail:

devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release

There are a few options:

1) Forget NuGet and just commit the referenced assemblies. This is the
simplest solution, but is more annoying to manage if we want to upgrade the
versions of these assemblies later.

2) Commit nuget.exe to the repo and just use it. This is simple enough, but
I'm not sure how people feel about committing binaries, and it would be
frozen at whatever version we commit.

3) Do some form of wget to grab the latest nuget.exe from "
https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
what's the simplest way to do this. Powershell could be used, but there are
security annoyances to deal with.

That's all I can come up with so far. Would love to hear from you guys if
you have any ideas or opinions on this. If you want I can send you a patch
of what I've got so far if that helps.

Thanks,

Antonio Maiorano



On Thu, 15 Sep 2016 at 19:35 Antonio Maiorano  wrote:

Sorry I haven't had a chance to get back to this. Things got busy at work.
I do plan to get back to this as I'm hoping to add some features to this
extension :)
On Thu, Sep 15, 2016 at 7:31 PM Zachary Turner  wrote:

Strange.  FWIW you can dump all the variables that are present in your
environment.  You need to go to Tools -> Options -> Projects and Solutions
-> Build and Run and choose either Normal, Detailed, or Diagnostic for the
MSBuild project build output verbosity.  Then in the output window you will
get a ton of spam, some of which is the set of all MSBuild variables you
can take advantage of.

On Thu, Sep 15, 2016 at 4:25 PM Hans Wennborg  wrote:

When I first opened the solution in VS it prompted me to install it and I
did.

On Thu, Sep 15, 2016 at 4:17 PM, Zachary Turner  wrote:
> You may need to install the Visual Studio SDK.  Did you do that when you
> initially installed VS 2015?
>
> On Thu, Sep 15, 2016 at 4:15 PM Hans Wennborg  wrote:
>>
>> Well, on my machine $(SDKToolsDir) doesn't work :-( I suspect the file
>> will need manual tweaking by whoever is trying to build the plugin.
>>
>> Anyway, I've updated the solution to build with VS2015 in r281648 and
>> confirmed that it can still be used with older VS versions too.
>>
>> Cheers,
>> Hans
>>
>> On Thu, Aug 18, 2016 at 7:11 PM, Zachary Turner 
>> wrote:
>> > The key.snk is generated when you build, the problem is the csproj file
>> > hardcodes the directory to the sdk instead of using the appropriate
>> > project
>> > system variable like $(SDKToolsDir)
>> >
>> > On Thu, Aug 18, 2016 at 7:09 PM Zachary Turner 
>> > wrote:
>> >>
>> >> Llvm doesn't support vs2012 anymore, as long as it supports vs2013
it's
>> >> fine
>> >> On Thu, Aug 18, 2016 at 7:07 PM Antonio Maiorano 
>> >> wrote:
>> >>>
>> >>> Hi,
>> >>>
>> >>> What I meant by upgrade was simply making it build in VS 2015.
>> >>> However,
>> >>> you bring up a valid point about making sure the extension will
>> >>> continue to
>> >>> work in VS 2012. I will look into that. Like those references that go
>> >>> from
>> >>> 10 to 14 that point out; I wonder if instead I should be able to
bring
>> >>> in
>> >>> those version 10 assemblies via NuGet. I'll take a closer look.
>> >>>
>> >>> Part of my change, however, seems to imply that the 

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Okay, that's fine, I'll go for that and if all looks good, will attach a
patch.

Thanks.

On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:

> I would use the first solution. We lock ourselves to specific versions of
> vs, so i think it's fine to do the same with the assemblies and deal with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>
> Hi Hans,
>
> I saw that on September 15th, you checked in a change: clang-format VS
> plugin: upgrade the project files to VS2015.
>
> When I open the latest version of ClangFormat.sln on a machine that has
> only VS 2015, it doesn't build. The reason is that some of the referenced
> assemblies are from VS 2010.
>
>   Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, Culture=neutral,
> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>
> What happens is that when building, these specific assemblies are not
> found. I suspect you have VS 2010 installed on your machine, which is why
> you don't get these build errors.
>
> The recommended way to handle this
> 
> is to make use of NuGet to have it automatically download the required
> assemblies. I have made the changes locally to get this working, and it
> works great when building ClangFormat.sln from within Visual Studio;
> however, building from the CLI doesn't work out of the box because you must
> explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
> (or devenv.exe in our case). The problem is that nuget.exe isn't something
> that's easily found/accessible on Windows, even once installed as an
> extension in VS. This is a known problem
>  and the current best solution
> requires downloading and making nuget.exe available in PATH
> .
>
> So now i'm faced with figuring out how best to solve this so that the
> custom build step in this CMakeLists.txt
> 
>  that
> runs devenv doesn't fail:
>
> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release
>
> There are a few options:
>
> 1) Forget NuGet and just commit the referenced assemblies. This is the
> simplest solution, but is more annoying to manage if we want to upgrade the
> versions of these assemblies later.
>
> 2) Commit nuget.exe to the repo and just use it. This is simple enough,
> but I'm not sure how people feel about committing binaries, and it would be
> frozen at whatever version we commit.
>
> 3) Do some form of wget to grab the latest nuget.exe from "
> https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
> what's the simplest way to do this. Powershell could be used, but there are
> security annoyances to deal with.
>
> That's all I can come up with so far. Would love to hear from you guys if
> you have any ideas or opinions on this. If you want I can send you a patch
> of what I've got so far if that helps.
>
> Thanks,
>
> Antonio Maiorano
>
>
>
> On Thu, 15 Sep 2016 at 19:35 Antonio Maiorano  wrote:
>
> Sorry I haven't had a chance to get back to this. Things got busy at work.
> I do plan to get back to this as I'm hoping to add some features to this
> extension :)
> On Thu, Sep 15, 2016 at 7:31 PM Zachary Turner  wrote:
>
> Strange.  FWIW you can dump all the variables that are present in your
> environment.  You need to go to Tools -> Options -> Projects and Solutions
> -> Build and Run and choose either Normal, Detailed, or Diagnostic for the
> MSBuild project build output verbosity.  Then in the output window you will
> get a ton of spam, some of which is the set of all MSBuild variables you
> can take advantage of.
>
> On Thu, Sep 15, 2016 at 4:25 PM Hans Wennborg  wrote:
>
> When I first opened the solution in VS it prompted me to install it and I
> did.
>
> On Thu, Sep 15, 2016 at 4:17 PM, Zachary Turner 
> wrote:
> > You may need to install the Visual Studio SDK.  Did you do that when you
> > initially installed VS 2015?
> >
> > On Thu, Sep 15, 2016 at 4:15 PM Hans Wennborg  wrote:
> >>
> >> Well, on my machine $(SDKToolsDir) doesn't work :-( I suspect the file
> >> will need manual tweaking by whoever is trying to build the plugin.
> >>
> >> Anyway, I've updated the solution to build with VS2015 in r281648 and
> >> confirmed that it can still be used with older VS versions too.
> >>
> >> Cheers,
> >> Hans
> >>
> >> On Thu, Aug 18, 2016 at 7:11 PM, Zachary Turner 
> >> wrote:
> >> > The key.snk is generated when you build, the problem is the csproj
> file
> >> > hardcodes the directory to the sdk instead of using the appropriate
> >> > project
> >> > system variable like $(SDKToolsDir)
> >> >
> >> > On Thu, Aug 18, 2016 at 

Re: Upgrade and fix clang-format-vs

[Antonio Maiorano via cfe-commits]

Hi again,

I've made the changes so that the required assemblies are committed, so now
we can build the clang-format-vsix with just VS 2015. Since the patch set
is around 9 mb, I'm providing a link to it on my Dropbox (if you'd rather I
attach it, let me know):

https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch

Note that it's a git patch set, for which I followed the instructions here
.

Thanks.

On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano  wrote:

> Okay, that's fine, I'll go for that and if all looks good, will attach a
> patch.
>
> Thanks.
>
> On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:
>
> I would use the first solution. We lock ourselves to specific versions of
> vs, so i think it's fine to do the same with the assemblies and deal with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>
> Hi Hans,
>
> I saw that on September 15th, you checked in a change: clang-format VS
> plugin: upgrade the project files to VS2015.
>
> When I open the latest version of ClangFormat.sln on a machine that has
> only VS 2015, it doesn't build. The reason is that some of the referenced
> assemblies are from VS 2010.
>
>   Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, Culture=neutral,
> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>
> What happens is that when building, these specific assemblies are not
> found. I suspect you have VS 2010 installed on your machine, which is why
> you don't get these build errors.
>
> The recommended way to handle this
> 
> is to make use of NuGet to have it automatically download the required
> assemblies. I have made the changes locally to get this working, and it
> works great when building ClangFormat.sln from within Visual Studio;
> however, building from the CLI doesn't work out of the box because you must
> explicitly run 'nuget.exe restore ClangFormat.sln' before running msbuild
> (or devenv.exe in our case). The problem is that nuget.exe isn't something
> that's easily found/accessible on Windows, even once installed as an
> extension in VS. This is a known problem
>  and the current best solution
> requires downloading and making nuget.exe available in PATH
> .
>
> So now i'm faced with figuring out how best to solve this so that the
> custom build step in this CMakeLists.txt
> 
>  that
> runs devenv doesn't fail:
>
> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release
>
> There are a few options:
>
> 1) Forget NuGet and just commit the referenced assemblies. This is the
> simplest solution, but is more annoying to manage if we want to upgrade the
> versions of these assemblies later.
>
> 2) Commit nuget.exe to the repo and just use it. This is simple enough,
> but I'm not sure how people feel about committing binaries, and it would be
> frozen at whatever version we commit.
>
> 3) Do some form of wget to grab the latest nuget.exe from "
> https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
> what's the simplest way to do this. Powershell could be used, but there are
> security annoyances to deal with.
>
> That's all I can come up with so far. Would love to hear from you guys if
> you have any ideas or opinions on this. If you want I can send you a patch
> of what I've got so far if that helps.
>
> Thanks,
>
> Antonio Maiorano
>
>
>
> On Thu, 15 Sep 2016 at 19:35 Antonio Maiorano  wrote:
>
> Sorry I haven't had a chance to get back to this. Things got busy at work.
> I do plan to get back to this as I'm hoping to add some features to this
> extension :)
> On Thu, Sep 15, 2016 at 7:31 PM Zachary Turner  wrote:
>
> Strange.  FWIW you can dump all the variables that are present in your
> environment.  You need to go to Tools -> Options -> Projects and Solutions
> -> Build and Run and choose either Normal, Detailed, or Diagnostic for the
> MSBuild project build output verbosity.  Then in the output window you will
> get a ton of spam, some of which is the set of all MSBuild variables you
> can take advantage of.
>
> On Thu, Sep 15, 2016 at 4:25 PM Hans Wennborg  wrote:
>
> When I first opened the solution in VS it prompted me to install it and I
> did.
>
> On Thu, Sep 15, 2016 at 4:17 PM, Zachary Turner 
> wrote:
> > You may need to install the Visual Studio SDK.  Did you do that when you
> > initially installed VS 2015?
> >
> > On Thu, Sep 15, 2016 at 4:15 PM Hans Wennborg  wrote:
> >>
> >> Well, on my machine $(SDKToolsDir) 

[PATCH] D26768: [analyzer] Improve VirtualCallChecker diagnostics and move out of alpha

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

Not sure if we should make pure vs not an option so that users could turn the 
checking off. Is there a way to suppress the warning?




Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:210
+  if (isPure)
+os << "pure ";
+

Please, add an intra-procedual test case for pure in we don't have one already.



Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:212
+
+  os << "virtual function during construction or destruction ";
+

Can we detect if we are in a constructor or if we are in the destructor and 
make the error message more precise?


https://reviews.llvm.org/D26768



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

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Adding the flag seems OK, but I'd rather not make the default setting be 
target-dependent, if that's workable.


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D26991: Hoist redundant load

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya updated this revision to Diff 79463.
hiraditya added a comment.

Removed unused code.


https://reviews.llvm.org/D26991

Files:
  libcxx/include/algorithm

Index: libcxx/include/algorithm
===
--- libcxx/include/algorithm
+++ libcxx/include/algorithm
@@ -1494,51 +1494,23 @@
 if (__len1 < __len2)
 return make_pair(__last1, __last1);
 const _RandomAccessIterator1 __s = __last1 - (__len2 - 1);  // Start of pattern match can't go beyond here
+
+// Load the first element from __first2 outside the loop because it is loop invariant
+typename iterator_traits<_RandomAccessIterator1>::value_type __firstElement2 = *__first2;
+
 while (true)
 {
-#if !_LIBCPP_UNROLL_LOOPS
 while (true)
 {
 if (__first1 == __s)
 return make_pair(__last1, __last1);
-if (__pred(*__first1, *__first2))
+if (__pred(*__first1, __firstElement2))
 break;
 ++__first1;
 }
-#else  // !_LIBCPP_UNROLL_LOOPS
-for (_D1 __loop_unroll = (__s - __first1) / 4; __loop_unroll > 0; --__loop_unroll)
-{
-if (__pred(*__first1, *__first2))
-goto __phase2;
-if (__pred(*++__first1, *__first2))
-goto __phase2;
-if (__pred(*++__first1, *__first2))
-goto __phase2;
-if (__pred(*++__first1, *__first2))
-goto __phase2;
-++__first1;
-}
-switch (__s - __first1)
-{
-case 3:
-if (__pred(*__first1, *__first2))
-break;
-++__first1;
-case 2:
-if (__pred(*__first1, *__first2))
-break;
-++__first1;
-case 1:
-if (__pred(*__first1, *__first2))
-break;
-case 0:
-return make_pair(__last1, __last1);
-}
-__phase2:
-#endif  // !_LIBCPP_UNROLL_LOOPS
+
 _RandomAccessIterator1 __m1 = __first1;
 _RandomAccessIterator2 __m2 = __first2;
-#if !_LIBCPP_UNROLL_LOOPS
  while (true)
  {
  if (++__m2 == __last2)
@@ -1550,43 +1522,6 @@
  break;
  }
  }
-#else  // !_LIBCPP_UNROLL_LOOPS
-++__m2;
-++__m1;
-for (_D2 __loop_unroll = (__last2 - __m2) / 4; __loop_unroll > 0; --__loop_unroll)
-{
-if (!__pred(*__m1, *__m2))
-goto __continue;
-if (!__pred(*++__m1, *++__m2))
-goto __continue;
-if (!__pred(*++__m1, *++__m2))
-goto __continue;
-if (!__pred(*++__m1, *++__m2))
-goto __continue;
-++__m1;
-++__m2;
-}
-switch (__last2 - __m2)
-{
-case 3:
-if (!__pred(*__m1, *__m2))
-break;
-++__m1;
-++__m2;
-case 2:
-if (!__pred(*__m1, *__m2))
-break;
-++__m1;
-++__m2;
-case 1:
-if (!__pred(*__m1, *__m2))
-break;
-case 0:
-return make_pair(__first1, __first1 + __len2);
-}
-__continue:
-++__first1;
-#endif  // !_LIBCPP_UNROLL_LOOPS
 }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Richard Smith via cfe-commits]

(Dropping Phabricator, since this isn't really about D27163...)

On 28 November 2016 at 14:27, John McCall via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> In https://reviews.llvm.org/D27163#607100, @rsmith wrote:
> > C has rather different and much less useful TBAA rules
>
> [...] I don't think the TBAA rules are as different as you're suggesting,


I can offhand think of at least the following differences, some of which
seem fairly major:
 * C does not allow changing the effective type of a declared variable, C++
does.
 * C does not support struct-path TBAA in any form (its model is that lumps
of memory have effective types, and it doesn't care how you got there), C++
does.
 * C allows any assignment to a union member to change the effective type,
no matter whether it's locally determinable that you're storing to a union
member, C++ requires the assignment's LHS to be a union member access.
 * C only permits common initial sequence shenanigans for unions where the
union type is visible, C++ permits it anywhere.


> except that C++ actually tries to provide specific rules for unions (that
> don't match what users actually want).


In what way don't C++'s union rules match user expectations where C's rules
do? Do you mean type punning via unions? C doesn't allow that either (or
maybe it does, if you think a footnote carries more normative weight than
rules in the main text).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26991: Hoist redundant load

[Aditya Kumar via Phabricator via cfe-commits]

hiraditya added a comment.

@mclow.lists 
I can remove this and update this patch with the load hoisted, if this is okay.


https://reviews.llvm.org/D26991



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

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL288083: IRGen: Remove all uses of CreateDefaultAlignedLoad. 
(authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D27157?vs=79436=79462#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27157

Files:
  cfe/trunk/lib/CodeGen/CGBuilder.h
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.cpp

Index: cfe/trunk/lib/CodeGen/CGBuilder.h
===
--- cfe/trunk/lib/CodeGen/CGBuilder.h
+++ cfe/trunk/lib/CodeGen/CGBuilder.h
@@ -124,19 +124,6 @@
   
   // FIXME: these "default-aligned" APIs should be removed,
   // but I don't feel like fixing all the builtin code right now.
-  llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr,
-   const llvm::Twine  = "") {
-return CGBuilderBaseTy::CreateLoad(Addr, false, Name);
-  }
-  llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr,
-   const char *Name) {
-return CGBuilderBaseTy::CreateLoad(Addr, false, Name);
-  }
-  llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, bool IsVolatile,
-   const llvm::Twine  = "") {
-return CGBuilderBaseTy::CreateLoad(Addr, IsVolatile, Name);
-  }
-
   llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val,
  llvm::Value *Addr,
  bool IsVolatile = false) {
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -2884,13 +2884,13 @@
   // FIXME: Generate IR in one pass, rather than going back and fixing up these
   // placeholders.
   llvm::Type *IRTy = CGF.ConvertTypeForMem(Ty);
-  llvm::Value *Placeholder =
-llvm::UndefValue::get(IRTy->getPointerTo()->getPointerTo());
-  Placeholder = CGF.Builder.CreateDefaultAlignedLoad(Placeholder);
+  llvm::Type *IRPtrTy = IRTy->getPointerTo();
+  llvm::Value *Placeholder = llvm::UndefValue::get(IRPtrTy->getPointerTo());
 
   // FIXME: When we generate this IR in one pass, we shouldn't need
   // this win32-specific alignment hack.
   CharUnits Align = CharUnits::fromQuantity(4);
+  Placeholder = CGF.Builder.CreateAlignedLoad(IRPtrTy, Placeholder, Align);
 
   return AggValueSlot::forAddr(Address(Placeholder, Align),
Ty.getQualifiers(),
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -2191,8 +2191,9 @@
 Value *IntToPtr =
   Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
  llvm::PointerType::get(IntTy, 257));
-LoadInst *Load =
-Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true);
+LoadInst *Load = Builder.CreateAlignedLoad(
+IntTy, IntToPtr, getContext().getTypeAlignInChars(E->getType()));
+Load->setVolatile(true);
 return RValue::get(Load);
   }
 
@@ -5440,9 +5441,11 @@
   switch (BuiltinID) {
   default: break;
   case NEON::BI__builtin_neon_vldrq_p128: {
-llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128);
+llvm::Type *Int128Ty = llvm::Type::getIntNTy(getLLVMContext(), 128);
+llvm::Type *Int128PTy = llvm::PointerType::get(Int128Ty, 0);
 Value *Ptr = Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PTy);
-return Builder.CreateDefaultAlignedLoad(Ptr);
+return Builder.CreateAlignedLoad(Int128Ty, Ptr,
+ CharUnits::fromQuantity(16));
   }
   case NEON::BI__builtin_neon_vstrq_p128: {
 llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128);
@@ -6615,27 +6618,37 @@
 return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "");
   }
   case NEON::BI__builtin_neon_vld1_v:
-  case NEON::BI__builtin_neon_vld1q_v:
+  case NEON::BI__builtin_neon_vld1q_v: {
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
-return Builder.CreateDefaultAlignedLoad(Ops[0]);
+auto Alignment = CharUnits::fromQuantity(
+BuiltinID == NEON::BI__builtin_neon_vld1_v ? 8 : 16);
+return Builder.CreateAlignedLoad(VTy, Ops[0], Alignment);
+  }
   case NEON::BI__builtin_neon_vst1_v:
   case NEON::BI__builtin_neon_vst1q_v:
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
 Ops[1] = Builder.CreateBitCast(Ops[1], VTy);
 return Builder.CreateDefaultAlignedStore(Ops[1], Ops[0]);
   case NEON::BI__builtin_neon_vld1_lane_v:
-  case NEON::BI__builtin_neon_vld1q_lane_v:
+  case NEON::BI__builtin_neon_vld1q_lane_v: {
 Ops[1] = Builder.CreateBitCast(Ops[1], 

r288083 - IRGen: Remove all uses of CreateDefaultAlignedLoad.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Mon Nov 28 16:30:21 2016
New Revision: 288083

URL: http://llvm.org/viewvc/llvm-project?rev=288083=rev
Log:
IRGen: Remove all uses of CreateDefaultAlignedLoad.

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

Modified:
cfe/trunk/lib/CodeGen/CGBuilder.h
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGBuilder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuilder.h?rev=288083=288082=288083=diff
==
--- cfe/trunk/lib/CodeGen/CGBuilder.h (original)
+++ cfe/trunk/lib/CodeGen/CGBuilder.h Mon Nov 28 16:30:21 2016
@@ -124,19 +124,6 @@ public:
   
   // FIXME: these "default-aligned" APIs should be removed,
   // but I don't feel like fixing all the builtin code right now.
-  llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr,
-   const llvm::Twine  = "") {
-return CGBuilderBaseTy::CreateLoad(Addr, false, Name);
-  }
-  llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr,
-   const char *Name) {
-return CGBuilderBaseTy::CreateLoad(Addr, false, Name);
-  }
-  llvm::LoadInst *CreateDefaultAlignedLoad(llvm::Value *Addr, bool IsVolatile,
-   const llvm::Twine  = "") {
-return CGBuilderBaseTy::CreateLoad(Addr, IsVolatile, Name);
-  }
-
   llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val,
  llvm::Value *Addr,
  bool IsVolatile = false) {

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=288083=288082=288083=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Nov 28 16:30:21 2016
@@ -2191,8 +2191,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 Value *IntToPtr =
   Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
  llvm::PointerType::get(IntTy, 257));
-LoadInst *Load =
-Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true);
+LoadInst *Load = Builder.CreateAlignedLoad(
+IntTy, IntToPtr, getContext().getTypeAlignInChars(E->getType()));
+Load->setVolatile(true);
 return RValue::get(Load);
   }
 
@@ -5440,9 +5441,11 @@ Value *CodeGenFunction::EmitAArch64Built
   switch (BuiltinID) {
   default: break;
   case NEON::BI__builtin_neon_vldrq_p128: {
-llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128);
+llvm::Type *Int128Ty = llvm::Type::getIntNTy(getLLVMContext(), 128);
+llvm::Type *Int128PTy = llvm::PointerType::get(Int128Ty, 0);
 Value *Ptr = Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), 
Int128PTy);
-return Builder.CreateDefaultAlignedLoad(Ptr);
+return Builder.CreateAlignedLoad(Int128Ty, Ptr,
+ CharUnits::fromQuantity(16));
   }
   case NEON::BI__builtin_neon_vstrq_p128: {
 llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128);
@@ -6615,27 +6618,37 @@ Value *CodeGenFunction::EmitAArch64Built
 return EmitNeonCall(CGM.getIntrinsic(Int, Tys), Ops, "");
   }
   case NEON::BI__builtin_neon_vld1_v:
-  case NEON::BI__builtin_neon_vld1q_v:
+  case NEON::BI__builtin_neon_vld1q_v: {
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
-return Builder.CreateDefaultAlignedLoad(Ops[0]);
+auto Alignment = CharUnits::fromQuantity(
+BuiltinID == NEON::BI__builtin_neon_vld1_v ? 8 : 16);
+return Builder.CreateAlignedLoad(VTy, Ops[0], Alignment);
+  }
   case NEON::BI__builtin_neon_vst1_v:
   case NEON::BI__builtin_neon_vst1q_v:
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
 Ops[1] = Builder.CreateBitCast(Ops[1], VTy);
 return Builder.CreateDefaultAlignedStore(Ops[1], Ops[0]);
   case NEON::BI__builtin_neon_vld1_lane_v:
-  case NEON::BI__builtin_neon_vld1q_lane_v:
+  case NEON::BI__builtin_neon_vld1q_lane_v: {
 Ops[1] = Builder.CreateBitCast(Ops[1], Ty);
 Ty = llvm::PointerType::getUnqual(VTy->getElementType());
 Ops[0] = Builder.CreateBitCast(Ops[0], Ty);
-Ops[0] = Builder.CreateDefaultAlignedLoad(Ops[0]);
+auto Alignment = CharUnits::fromQuantity(
+BuiltinID == NEON::BI__builtin_neon_vld1_lane_v ? 8 : 16);
+Ops[0] =
+Builder.CreateAlignedLoad(VTy->getElementType(), Ops[0], Alignment);
 return Builder.CreateInsertElement(Ops[1], Ops[0], Ops[2], "vld1_lane");
+  }
   case NEON::BI__builtin_neon_vld1_dup_v:
   case NEON::BI__builtin_neon_vld1q_dup_v: {
 Value *V = UndefValue::get(Ty);
 Ty = 

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/Frontend/FrontendActions.cpp:227
+IsBuiltin = true;
+  addHeaderInclude(H.NameAsWritten, Includes, LangOpts, Module->IsExternC,
+   IsBuiltin /*AlwaysInclude*/);

I don't understand why this would be necessary. If these headers are in fact 
textual headers, they won't be in the `HK_Normal` or `HK_Private` lists at all 
(they'll be in the `HK_Private` or `HK_PrivateTextual` lists instead), so your 
`IsBuiltin = true;` line should be unreachable. (Checking for an absolute path 
also seems suspicious here.)

I suspect the problem is instead that `#import` just doesn't work properly with 
modules (specifically, either with local submodule visibility or with modules 
that do not `export *`), because it doesn't care whether the previous inclusion 
is visible or not, and as a result it assumes "I've heard about someone 
#including this ever" means the same thing as "the contents of this file are 
visible right now". I know that `#pragma once` has this issue, so I'd expect 
`#import` to also suffer from it.


https://reviews.llvm.org/D26267



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

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuilder.h:126
   // FIXME: these "default-aligned" APIs should be removed,
   // but I don't feel like fixing all the builtin code right now.
   llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val,

pcc wrote:
> rjmccall wrote:
> > Oh, please remove this comment, too, since you've now achieved it. :)
> We still have CreateDefaultAlignedStore though.
Oh, yes, of course.  In that case, this LGTM.


https://reviews.llvm.org/D27157



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

r288080 - Hide the result of building a constant initializer. NFC.

[John McCall via cfe-commits]

Author: rjmccall
Date: Mon Nov 28 16:18:30 2016
New Revision: 288080

URL: http://llvm.org/viewvc/llvm-project?rev=288080=rev
Log:
Hide the result of building a constant initializer.  NFC.

Modified:
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/ConstantBuilder.h

Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=288080=288079=288080=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Mon Nov 28 16:18:30 2016
@@ -1549,7 +1549,7 @@ GenerateMethodList(StringRef ClassName,
 Methods.add(
 llvm::ConstantStruct::get(ObjCMethodTy, {C, MethodTypes[i], Method}));
   }
-  MethodList.add(Methods.finish());
+  Methods.finishAndAddTo(MethodList);
 
   // Create an instance of the structure
   return MethodList.finishAndCreateGlobal(".objc_method_list",
@@ -1584,9 +1584,9 @@ GenerateIvarList(ArrayRefsecond);
@@ -2498,7 +2498,7 @@ llvm::Function *CGObjCGNU::ModuleInitFun
 auto SelStruct = Selectors.beginStruct(SelStructTy);
 SelStruct.add(NULLPtr);
 SelStruct.add(NULLPtr);
-Selectors.add(SelStruct.finish());
+SelStruct.finishAndAddTo(Selectors);
   }
 
   // Number of static selectors

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=288080=288079=288080=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Nov 28 16:18:30 2016
@@ -2832,7 +2832,7 @@ CGOpenMPRuntime::createOffloadingBinaryD
 Dev.add(ImgEnd);
 Dev.add(HostEntriesBegin);
 Dev.add(HostEntriesEnd);
-DeviceImagesEntries.add(Dev.finish());
+Dev.finishAndAddTo(DeviceImagesEntries);
   }
 
   // Create device images global array.

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=288080=288079=288080=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Nov 28 16:18:30 2016
@@ -753,7 +753,7 @@ void CodeGenModule::EmitCtorList(CtorLis
   ctor.add(llvm::ConstantExpr::getBitCast(I.AssociatedData, VoidPtrTy));
 else
   ctor.addNullPointer(VoidPtrTy);
-ctors.add(ctor.finish());
+ctor.finishAndAddTo(ctors);
   }
 
   auto list =

Modified: cfe/trunk/lib/CodeGen/ConstantBuilder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ConstantBuilder.h?rev=288080=288079=288080=diff
==
--- cfe/trunk/lib/CodeGen/ConstantBuilder.h (original)
+++ cfe/trunk/lib/CodeGen/ConstantBuilder.h Mon Nov 28 16:18:30 2016
@@ -58,10 +58,10 @@ public:
 assert(Buffer.empty() && "didn't claim all values out of buffer");
   }
 
-  class AggregateBuilder {
+  class AggregateBuilderBase {
   protected:
 ConstantInitBuilder 
-AggregateBuilder *Parent;
+AggregateBuilderBase *Parent;
 size_t Begin;
 bool Finished = false;
 bool Frozen = false;
@@ -74,8 +74,8 @@ public:
   return Builder.Buffer;
 }
 
-AggregateBuilder(ConstantInitBuilder ,
- AggregateBuilder *parent)
+AggregateBuilderBase(ConstantInitBuilder ,
+ AggregateBuilderBase *parent)
 : Builder(builder), Parent(parent), Begin(builder.Buffer.size()) {
   if (parent) {
 assert(!parent->Frozen && "parent already has child builder active");
@@ -86,7 +86,7 @@ public:
   }
 }
 
-~AggregateBuilder() {
+~AggregateBuilderBase() {
   assert(Finished && "didn't claim value from aggregate builder");
 }
 
@@ -107,17 +107,17 @@ public:
 
   public:
 // Not copyable.
-AggregateBuilder(const AggregateBuilder &) = delete;
-AggregateBuilder =(const AggregateBuilder &) = delete;
+AggregateBuilderBase(const AggregateBuilderBase &) = delete;
+AggregateBuilderBase =(const AggregateBuilderBase &) = delete;
 
 // Movable, mostly to allow returning.  But we have to write this out
 // properly to satisfy the assert in the destructor.
-AggregateBuilder(AggregateBuilder &)
+AggregateBuilderBase(AggregateBuilderBase &)
   : Builder(other.Builder), Parent(other.Parent), Begin(other.Begin),
 Finished(other.Finished), Frozen(other.Frozen) {
   other.Finished = false;
 }
-AggregateBuilder =(AggregateBuilder &) = delete;
+AggregateBuilderBase =(AggregateBuilderBase &) = delete;
 
 void add(llvm::Constant *value) {
   assert(!Finished && "cannot add more values after 

r288079 - ConstantBuilder -> ConstantInitBuilder for clarity, and

[John McCall via cfe-commits]

Author: rjmccall
Date: Mon Nov 28 16:18:27 2016
New Revision: 288079

URL: http://llvm.org/viewvc/llvm-project?rev=288079=rev
Log:
ConstantBuilder -> ConstantInitBuilder for clarity, and
move the member classes up to top level to allow forward
declarations to name them.  NFC.

Modified:
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGCUDANV.cpp
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/ConstantBuilder.h

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=288079=288078=288079=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Mon Nov 28 16:18:27 2016
@@ -88,7 +88,7 @@ static llvm::Constant *buildBlockDescrip
   else
 i8p = CGM.VoidPtrTy;
 
-  ConstantBuilder builder(CGM);
+  ConstantInitBuilder builder(CGM);
   auto elements = builder.beginStruct();
 
   // reserved
@@ -1076,7 +1076,7 @@ static llvm::Constant *buildGlobalBlock(
   assert(blockInfo.CanBeGlobal);
 
   // Generate the constants for the block literal initializer.
-  ConstantBuilder builder(CGM);
+  ConstantInitBuilder builder(CGM);
   auto fields = builder.beginStruct();
 
   // isa

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=288079=288078=288079=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Mon Nov 28 16:18:27 2016
@@ -298,7 +298,7 @@ llvm::Function *CGNVCUDARuntime::makeMod
 CGM.getTriple().isMacOSX() ? "__NV_CUDA,__fatbin" : ".nvFatBinSegment";
 
 // Create initialized wrapper structure that points to the loaded GPU 
binary
-ConstantBuilder Builder(CGM);
+ConstantInitBuilder Builder(CGM);
 auto Values = Builder.beginStruct(FatbinWrapperTy);
 // Fatbin wrapper magic.
 Values.addInt(IntTy, 0x466243b1);

Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=288079=288078=288079=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Mon Nov 28 16:18:27 2016
@@ -222,7 +222,7 @@ protected:
   }
 
   /// Push the property attributes into two structure fields. 
-  void PushPropertyAttributes(ConstantBuilder::StructBuilder ,
+  void PushPropertyAttributes(ConstantStructBuilder ,
   ObjCPropertyDecl *property, bool isSynthesized=true, bool
   isDynamic=true) {
 int attrs = property->getPropertyAttributes();
@@ -1204,7 +1204,7 @@ llvm::Constant *CGObjCGNUstep::GetEHType
   llvm::Constant *typeName =
 ExportUniqueString(className, "__objc_eh_typename_");
 
-  ConstantBuilder builder(CGM);
+  ConstantInitBuilder builder(CGM);
   auto fields = builder.beginStruct();
   fields.add(BVtable);
   fields.add(typeName);
@@ -1242,7 +1242,7 @@ ConstantAddress CGObjCGNU::GenerateConst
   else if (isa->getType() != PtrToIdTy)
 isa = llvm::ConstantExpr::getBitCast(isa, PtrToIdTy);
 
-  ConstantBuilder Builder(CGM);
+  ConstantInitBuilder Builder(CGM);
   auto Fields = Builder.beginStruct();
   Fields.add(isa);
   Fields.add(MakeConstantString(Str));
@@ -1524,7 +1524,7 @@ GenerateMethodList(StringRef ClassName,
   if (MethodSels.empty())
 return NULLPtr;
 
-  ConstantBuilder Builder(CGM);
+  ConstantInitBuilder Builder(CGM);
 
   auto MethodList = Builder.beginStruct();
   MethodList.addNullPointer(CGM.Int8PtrTy);
@@ -1564,7 +1564,7 @@ GenerateIvarList(ArrayRef Protocols) {
 
-  ConstantBuilder Builder(CGM);
+  ConstantInitBuilder Builder(CGM);
   auto ProtocolList = Builder.beginStruct();
   ProtocolList.add(NULLPtr);
   ProtocolList.addInt(LongTy, Protocols.size());
@@ -1770,7 +1770,7 @@ CGObjCGNU::GenerateEmptyProtocol(const s
   llvm::Constant *MethodList = GenerateProtocolMethodList({}, {});
   // Protocols are objects containing lists of the methods implemented and
   // protocols adopted.
-  ConstantBuilder Builder(CGM);
+  ConstantInitBuilder Builder(CGM);
   auto Elements = Builder.beginStruct();
 
   // The isa pointer must be set to a magic number so the runtime knows it's
@@ -1869,13 +1869,13 @@ void CGObjCGNU::GenerateProtocol(const O
 numReqProperties++;
 }
 
-ConstantBuilder reqPropertyListBuilder(CGM);
+ConstantInitBuilder reqPropertyListBuilder(CGM);
 auto reqPropertiesList = reqPropertyListBuilder.beginStruct();
 reqPropertiesList.addInt(IntTy, numReqProperties);
 reqPropertiesList.add(NULLPtr);
 auto reqPropertiesArray = reqPropertiesList.beginArray(propertyMetadataTy);
 
-ConstantBuilder optPropertyListBuilder(CGM);
+

r288081 - Make CGVTables use ConstantInitBuilder. NFC.

[John McCall via cfe-commits]

Author: rjmccall
Date: Mon Nov 28 16:18:33 2016
New Revision: 288081

URL: http://llvm.org/viewvc/llvm-project?rev=288081=rev
Log:
Make CGVTables use ConstantInitBuilder.  NFC.

Modified:
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CGVTables.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=288081=288080=288081=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Mon Nov 28 16:18:33 2016
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
+#include "ConstantBuilder.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
@@ -524,29 +525,29 @@ void CodeGenVTables::EmitThunks(GlobalDe
 emitThunk(GD, Thunk, /*ForVTable=*/false);
 }
 
-llvm::Constant *CodeGenVTables::CreateVTableComponent(
-unsigned Idx, const VTableLayout , llvm::Constant *RTTI,
-unsigned ) {
-  VTableComponent Component = VTLayout.vtable_components()[Idx];
-
-  auto OffsetConstant = [&](CharUnits Offset) {
-return llvm::ConstantExpr::getIntToPtr(
-llvm::ConstantInt::get(CGM.PtrDiffTy, Offset.getQuantity()),
-CGM.Int8PtrTy);
+void CodeGenVTables::addVTableComponent(
+ConstantArrayBuilder , const VTableLayout ,
+unsigned idx, llvm::Constant *rtti, unsigned ) {
+  auto  = layout.vtable_components()[idx];
+
+  auto addOffsetConstant = [&](CharUnits offset) {
+builder.add(llvm::ConstantExpr::getIntToPtr(
+llvm::ConstantInt::get(CGM.PtrDiffTy, offset.getQuantity()),
+CGM.Int8PtrTy));
   };
 
-  switch (Component.getKind()) {
+  switch (component.getKind()) {
   case VTableComponent::CK_VCallOffset:
-return OffsetConstant(Component.getVCallOffset());
+return addOffsetConstant(component.getVCallOffset());
 
   case VTableComponent::CK_VBaseOffset:
-return OffsetConstant(Component.getVBaseOffset());
+return addOffsetConstant(component.getVBaseOffset());
 
   case VTableComponent::CK_OffsetToTop:
-return OffsetConstant(Component.getOffsetToTop());
+return addOffsetConstant(component.getOffsetToTop());
 
   case VTableComponent::CK_RTTI:
-return RTTI;
+return builder.add(llvm::ConstantExpr::getBitCast(rtti, CGM.Int8PtrTy));
 
   case VTableComponent::CK_FunctionPointer:
   case VTableComponent::CK_CompleteDtorPointer:
@@ -554,17 +555,17 @@ llvm::Constant *CodeGenVTables::CreateVT
 GlobalDecl GD;
 
 // Get the right global decl.
-switch (Component.getKind()) {
+switch (component.getKind()) {
 default:
   llvm_unreachable("Unexpected vtable component kind");
 case VTableComponent::CK_FunctionPointer:
-  GD = Component.getFunctionDecl();
+  GD = component.getFunctionDecl();
   break;
 case VTableComponent::CK_CompleteDtorPointer:
-  GD = GlobalDecl(Component.getDestructorDecl(), Dtor_Complete);
+  GD = GlobalDecl(component.getDestructorDecl(), Dtor_Complete);
   break;
 case VTableComponent::CK_DeletingDtorPointer:
-  GD = GlobalDecl(Component.getDestructorDecl(), Dtor_Deleting);
+  GD = GlobalDecl(component.getDestructorDecl(), Dtor_Deleting);
   break;
 }
 
@@ -580,68 +581,69 @@ llvm::Constant *CodeGenVTables::CreateVT
   ? MD->hasAttr()
   : (MD->hasAttr() || 
!MD->hasAttr());
   if (!CanEmitMethod)
-return llvm::ConstantExpr::getNullValue(CGM.Int8PtrTy);
+return builder.addNullPointer(CGM.Int8PtrTy);
   // Method is acceptable, continue processing as usual.
 }
 
-auto SpecialVirtualFn = [&](llvm::Constant *, StringRef Name) {
-  if (!Cache) {
-llvm::FunctionType *Ty =
-llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
-Cache = CGM.CreateRuntimeFunction(Ty, Name);
-if (auto *F = dyn_cast(Cache))
-  F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);
-  }
-  return Cache;
+auto getSpecialVirtualFn = [&](StringRef name) {
+  llvm::FunctionType *fnTy =
+  llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
+  llvm::Constant *fn = CGM.CreateRuntimeFunction(fnTy, name);
+  if (auto f = dyn_cast(fn))
+f->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+  return llvm::ConstantExpr::getBitCast(fn, CGM.Int8PtrTy);
 };
 
-if (cast(GD.getDecl())->isPure())
-  // We have a pure virtual member function.
-  return SpecialVirtualFn(PureVirtualFn,
-  CGM.getCXXABI().GetPureVirtualCallName());
-
-if (cast(GD.getDecl())->isDeleted())
-  return SpecialVirtualFn(DeletedVirtualFn,
-   

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D27163#607100, @rsmith wrote:

> In https://reviews.llvm.org/D27163#607078, @rsmith wrote:
>
> > A target-specific default for this, simply because there's a lot of code on 
> > Darwin that happens to violate this language rule, doesn't make sense to me.
>
>
> ... but rjmccall's explanation of the problem helps. The C compatibility 
> argument also suggests that this should only apply to trivially-copyable 
> types, and perhaps only scalar types.


I would agree with this.  The right rule is probably whether the type is 
trivially-destructed.

> The same issue presumably arises for `-fstrict-enums`, which is again only UB 
> in C++? (Also, C has rather different and much less useful TBAA rules.) 
> Perhaps some catch-all "I want defined behavior for things that C defines 
> even though I'm compiling in C++" flag would make sense here?

I don't think we've seen problems from any of these.  Also I don't think the 
TBAA rules are as different as you're suggesting, except that C++ actually 
tries to provide specific rules for unions (that don't match what users 
actually want).


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

@rsmith ping!


https://reviews.llvm.org/D26267



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

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D27163#607078, @rsmith wrote:

> A target-specific default for this, simply because there's a lot of code on 
> Darwin that happens to violate this language rule, doesn't make sense to me.


... but rjmccall's explanation of the problem helps. The C compatibility 
argument also suggests that this should only apply to trivially-copyable types, 
and perhaps only scalar types. The same issue presumably arises for 
`-fstrict-enums`, which is again only UB in C++? (Also, C has rather different 
and much less useful TBAA rules.) Perhaps some catch-all "I want defined 
behavior for things that C defines even though I'm compiling in C++" flag would 
make sense here?


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:8064
+  ScalarCast = CK_FloatingCast;
+} else if (ScalarTy->isIntegralType(S.Context)) {
+  // Determine if the integer constant can be expressed as a floating point

sdardis wrote:
> bruno wrote:
> > I don't see why it's necessary to check for all specific cases where the 
> > scalar is a constant. For all the others scenarios it should be enough to 
> > get the right answer via `getIntegerTypeOrder` or `getFloatTypeOrder`. For 
> > this is specific condition, the `else` part for the `CstScalar` below 
> > should also handle the constant case, right? 
> > 
> > 
> If we have a scalar constant, it is necessary to examine it's actual value to 
> see if it can be casted without truncation. The scalar constant's type is 
> somewhat irrelevant. Consider:
> 
>v4f32 f(v4f32 a) {
>  return a + (double)1.0;
>}
> 
> In this case examining the order only works if the vector's order is greater 
> than or equal to the scalar constant's order. Instead, if we ask whether the 
> scalar constant can be represented as a constant scalar of the vector's 
> element type, then we know if we can convert without the loss of precision.
> 
> For integers, the case is a little more contrived due to native integer 
> width, but the question is the same. Can a scalar constant X be represented 
> as a given floating point type. Consider:
> 
>v4f32 f(v4f32 a) {
>  return a + (signed int)1;
> }
> 
> This would rejected for platforms where a signed integer's width is greater 
> than the precision of float if we examined it based purely on types and their 
> sizes. Instead, if we convert the integer to the float point's type with 
> APFloat and convert back to see if we have the same value, we can determine 
> if the scalar constant can be safely converted without the loss of precision.
> 
> I based the logic examining the value of the constant scalar based on GCC's 
> behaviour, which implicitly converts scalars regardless of their type if they 
> can be represented in the same type of the vector's element type without the 
> loss of precision. If the scalar cannot be evaluated to a constant at compile 
> time, then the size in bits for the scalar's type determines if it can be 
> converted safely.
Right. Can you split the scalarTy <-> vectorEltTy checking logic into separate 
functions; one for cst-scalar-int-to-vec-elt-int and another for 
scalar-int-to-vec-elt-float? 



Comment at: lib/Sema/SemaExpr.cpp:8267
+  }
+
   // Otherwise, use the generic diagnostic.

sdardis wrote:
> bruno wrote:
> > This change seems orthogonal to this patch. Can you make it a separated 
> > patch with the changes from test/Sema/vector-cast.c?
> This change is a necessary part of this patch and it can't be split out in 
> sensible fashion.
> 
> For example, line 329 of test/Sema/zvector.c adds a scalar signed character 
> to a vector of signed characters. With just this change we would report 
> "cannot convert between scalar type 'signed char' and vector type '__vector 
> signed char' (vector of 16 'signed char' values) as implicit conversion would 
> cause truncation".
> 
> This is heavily misleading for C and C++ code as we don't perform implicit 
> conversions and signed char could be implicitly converted to a vector of 
> signed char without the loss of precision.
> 
> To make this diagnostic precise without performing implicit conversions 
> requires determining cases where implicit conversion would cause truncation 
> and reporting that failure reason, or defaulting to the generic diagnostic.
> 
> Keeping this change as part of this patch is cleaner and simpler as it covers 
> both implicit conversions and more precise diagnostics.
Can you double check whether we should support these GCC semantics for 
getLangOpts().ZVector? If not, then this should be introduced in a separate 
patch/commit.



Comment at: lib/Sema/SemaExpr.cpp:8020
+  if (Order < 0)
+return true;
+}

Please also early return `!CstScalar && Order < 0` like you did below.



Comment at: lib/Sema/SemaExpr.cpp:8064
+ !ScalarTy->hasSignedIntegerRepresentation());
+bool ignored = false;
+Float.convertToInteger(

`ignored` => `Ignored` 



Comment at: lib/Sema/SemaExpr.cpp:8171
+return LHSType;
+}
   }

It's not clear to me whether clang should support the GCC semantics when in 
`getLangOpts().AltiVec || getLangOpts().ZVector`, do you?

You can remove the curly braces for the `if` and `else` here.



Comment at: lib/Sema/SemaExpr.cpp:8182
+return RHSType;
+}
   }

Also remove braces here.


https://reviews.llvm.org/D25866



___
cfe-commits mailing list

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

A target-specific default for this, simply because there's a lot of code on 
Darwin that happens to violate this language rule, doesn't make sense to me.

Basing the behavior on whether a `-Wreturn-type` warning would have been 
emitted seems like an extremely strange heuristic: only optimizing in the cases 
where we provide the user no hint that we will do so seems incredibly 
user-hostile.

Regardless of anything else, it does not make any sense to "return" stack 
garbage when the return type is a C++ class type, particularly one with a 
non-trivial copy constructor or destructor. A trap at `-O0` is the kindest 
thing we can do in that case.

In summary, I think it could be reasonable to have such a flag to disable the 
trap/unreachable *only* for scalar types (or, at a push, trivially-copyable 
types). But it seems unreasonable to have different defaults for Darwin, or to 
look at whether a `-Wreturn-type` warning would have fired.

Alternatively, since it seems you're only interested in the behavior of cases 
where `-Wreturn-type` would have fired, how about using Clang's tooling support 
to write a utility to add a return statement to the end of every function where 
it would fire, and give that to your users to fix their code?


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: test/CodeGenCXX/return.cpp:21
+  // CHECK-NOSTRICT-NEXT: load
+  // CHECK-NOSTRICT-NEXT: ret i32
+  // CHECK-NOSTRICT-NEXT: }

rjmccall wrote:
> mehdi_amini wrote:
> > Quuxplusone wrote:
> > > Can you explain why a load from an uninitialized stack location would be 
> > > *better* than a trap and/or `unreachable`? IIUC this is basically what 
> > > Mehdi is asking: i.e., can you explain the rationale for this patch, 
> > > because I don't get it either. It *seems* like a strict regression in 
> > > code quality.
> > There is a difference. LLVM will optimize:
> > 
> > ```
> > define i32 @foo() {
> >   %1 = alloca i32, align 4
> >   %2 = load i32, i32* %1, align 4
> >   ret i32 %2
> > }
> > ```
> > 
> > to:
> > 
> > ```
> > define i32 @foo() {
> >   ret i32 undef
> > }
> > ```
> > 
> > Which means "return an undefined value" (basically any valide bit-pattern 
> > for an i32). This is not undefined behavior if the caller does not use the 
> > value with a side-effect.
> > 
> > However with:
> > 
> > ```
> > define i32 @foo() {
> >   unreachable
> > }
> > ```
> > 
> > Just calling `foo()` is undefined behavior, even if the returned value 
> > isn't used.
> > 
> > So what this patch does is actually making the compiled-code `safer` by 
> > inhibiting some optimizations based on this UB. 
> We've been disabling this optimization completely in Apple compilers for 
> years, so allowing it to ever kick in is actually an improvement.  I'll try 
> to elaborate on our reasoning for this change, which *is* actually somewhat 
> Apple-specific.
> 
> Falling off the end of a non-void function is only undefined behavior in C++, 
> not in C.  It is fairly common practice to compile C code as C++, and while 
> there's a large number of potential language incompatibilities there, they 
> tend to be rare or trivial to fix in practice.  At Apple, we have a specific 
> need to compile C code as C++ because of the C++-based IOKit API: while 
> drivers are overwhelmingly written as C code, at Apple they have to be 
> compiled as C++ in order to talk to the kernel.  Moreover, often Apple did 
> not write the drivers in question, and maintaining a large set of patches in 
> order to eliminate undefined behavior that wasn't actually UB in the 
> originally-intended build configuration is not really seen as acceptable.
> 
> It makes sense to me to expose this as a flag akin to -fno-strict-aliasing in 
> order to support C-in-C++ code bases like Apple's drivers.  It is possible 
> that we don't actually have to change the default for the flag on Apple 
> platforms and can instead pursue more targeted build changes.
I totally understand why such flag is desirable, it is just not a clear cut to 
make it the default on one platform only (and having this flag available 
upstream does not prevent the Xcode clang from enabling this by default though, 
if fixing the build settings isn't possible/desirable). 


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Sean Callanan via Phabricator via cfe-commits]

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

Yeah, that test looks great!  Thanks!




Comment at: lib/AST/ASTImporter.cpp:496
+return false;
+  if (DN1->isIdentifier())
+return IsStructurallyEquivalent(DN1->getIdentifier(),

spyffe wrote:
> We should probably also check whether `DN1->isIdentifier() == 
> DN2->isIdentifier()`.
Looking at my comment with fresh post Thanksgiving eyes, that would be totally 
wrong.  The `IsStructurallyEquivalent` is fine.



Comment at: 
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:67
+template
+struct Child1: public Two::Three::Parent {
+  char member;

ooh, nice!


https://reviews.llvm.org/D26753



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

[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/Basic/TargetInfo.cpp:229
   switch (BitWidth) {
-  case 96:
+  case 80:
 if (() == ::APFloat::x87DoubleExtended)

ddcc wrote:
> bruno wrote:
> > The change makes sense but I believe there's some historical reason why 
> > this is 96 instead of 80? What problem have you found in practice? Do you 
> > have a testcase or unittest that exercise the issue (and that could be 
> > added to the patch)?
> I'd be curious why it was historically set to 96; I dug up rL190044 as the 
> original commit, but it doesn't mention 80 vs 96 for this at all.
> 
> I've been implementing an alternative symbolic constraint solver backend for 
> the static analyzer, including floating-point support, which performs some 
> type conversion and needs to reason about bitwidth. I'm still working on 
> those patches, since they touch a lot of code and currently break some tests. 
> I can write up a testcase for this issue, though I've only written IR 
> testcases before and I'm not sure how I'd directly call a clang internal 
> function?
It was set to 96 because that's what the only caller (handleModeAttr) expects; 
see https://reviews.llvm.org/rL65935 and https://reviews.llvm.org/rL190926.  
It's arbitrary as far as I know.


https://reviews.llvm.org/D26955



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

[PATCH] D27033: [ASTImporter] Support importing UnresolvedLookupExpr nodes

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

I only have a stylistic nit to add to Aleksei's comments.




Comment at: lib/AST/ASTImporter.cpp:6492
+  UnresolvedSet<8> ToDecls;
+  for (UnresolvedLookupExpr::decls_iterator S = E->decls_begin(),
+F = E->decls_end();

a.sidorin wrote:
> `auto` will look nice here.
Alternatively,
```
for (Decl *D : E->decls())
```


https://reviews.llvm.org/D27033



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

[PATCH] D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters

[Matt Bettinson via Phabricator via cfe-commits]

bettinson added a comment.

ping


https://reviews.llvm.org/D26800



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

[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/Basic/TargetInfo.cpp:229
   switch (BitWidth) {
-  case 96:
+  case 80:
 if (() == ::APFloat::x87DoubleExtended)

bruno wrote:
> The change makes sense but I believe there's some historical reason why this 
> is 96 instead of 80? What problem have you found in practice? Do you have a 
> testcase or unittest that exercise the issue (and that could be added to the 
> patch)?
I'd be curious why it was historically set to 96; I dug up rL190044 as the 
original commit, but it doesn't mention 80 vs 96 for this at all.

I've been implementing an alternative symbolic constraint solver backend for 
the static analyzer, including floating-point support, which performs some type 
conversion and needs to reason about bitwidth. I'm still working on those 
patches, since they touch a lot of code and currently break some tests. I can 
write up a testcase for this issue, though I've only written IR testcases 
before and I'm not sure how I'd directly call a clang internal function?


https://reviews.llvm.org/D26955



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

[PATCH] D26904: [astimporter] Support importing CXXDependentScopeMemberExpr and FunctionTemplateDecl

[Sean Callanan via Phabricator via cfe-commits]

spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.

There are several missing imports here, as well as a few minor nits.
If the unit test cases aren't catching these, I'm a little concerned.  We 
should be catching this.
Also we should definitely test that bodies of function templates (in 
particular, bodies that use the template arguments) get imported properly.




Comment at: lib/AST/ASTImporter.cpp:2327
+void ASTNodeImporter::ImportAttributes(Decl *From, Decl *To) {
+  for (Decl::attr_iterator I = From->attr_begin(), E = From->attr_end(); I != 
E;
+   ++I) {

Would
```
for (Attr *A : From->atrs()) {
```
work in this case?



Comment at: lib/AST/ASTImporter.cpp:3564
+
+  FunctionTemplateDecl *ToFunc = FunctionTemplateDecl::Create(
+  Importer.getToContext(), DC, Loc, Name, Params, TemplatedFD);

You didn't import `TemplatedFD` before installing it in `ToFunc`.  This code is 
broken, and we should make sure the unit tests know to catch these cases.



Comment at: lib/AST/ASTImporter.cpp:6482
+  return CXXDependentScopeMemberExpr::Create(Importer.getToContext(),
+ Base, BaseType,
+ E->isArrow(),

You're installing `Base` and `BaseType` without importing them, as well as all 
the `Loc`s.


https://reviews.llvm.org/D26904



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

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: lib/Sema/AnalysisBasedWarnings.cpp:530
+  // Don't need to mark Objective-C methods or blocks since the undefined
+  // behaviour optimization isn't used for them.
+}

Quuxplusone wrote:
> This seems like a trap waiting to spring on someone, unless there's a 
> technical reason that methods and blocks cannot possibly use the same 
> optimization paths as regular functions. ("Nobody's gotten around to 
> implementing it yet" is the most obvious nontechnical reason for the current 
> difference.) Either way, I'd expect this patch to include test cases for both 
> methods and blocks, to verify that the behavior you expect is actually the 
> behavior that happens. Basically, it ought to have a regression test 
> targeting the regression that I'm predicting is going to spring on someone as 
> soon as they implement optimizations for methods and blocks.
> 
> Also, one dumb question: what about C++ lambdas? are they FunctionDecls too? 
> test case?
> This seems like a trap waiting to spring on someone, unless there's a 
> technical reason that methods and blocks cannot possibly use the same 
> optimization paths as regular functions.

The optimization path in LLVM is the same. I think the difference lies in clang 
IRGen: there is no "unreachable" generated for these so the optimizer can't be 
aggressive. So this patch is not changing anything for Objective-C methods and 
blocks, and I expect that we *already* have a test that covers this behavior 
(if not we should add one).



Comment at: test/CodeGenCXX/return.cpp:21
+  // CHECK-NOSTRICT-NEXT: load
+  // CHECK-NOSTRICT-NEXT: ret i32
+  // CHECK-NOSTRICT-NEXT: }

Quuxplusone wrote:
> Can you explain why a load from an uninitialized stack location would be 
> *better* than a trap and/or `unreachable`? IIUC this is basically what Mehdi 
> is asking: i.e., can you explain the rationale for this patch, because I 
> don't get it either. It *seems* like a strict regression in code quality.
There is a difference. LLVM will optimize:

```
define i32 @foo() {
  %1 = alloca i32, align 4
  %2 = load i32, i32* %1, align 4
  ret i32 %2
}
```

to:

```
define i32 @foo() {
  ret i32 undef
}
```

Which means "return an undefined value" (basically any valide bit-pattern for 
an i32). This is not undefined behavior if the caller does not use the value 
with a side-effect.

However with:

```
define i32 @foo() {
  unreachable
}
```

Just calling `foo()` is undefined behavior, even if the returned value isn't 
used.

So what this patch does is actually making the compiled-code `safer` by 
inhibiting some optimizations based on this UB. 



Comment at: test/CodeGenCXX/return.cpp:35
+// CHECK-NOSTRICT: @_Z21alwaysOptimizedReturn4Enum
+// CHECK-NOSTRICT-OPT: @_Z21alwaysOptimizedReturn4Enum
+int alwaysOptimizedReturn(Enum e) {

This should be `-LABEL` check lines. And instead of repeating 4 times the same 
check, you could add a common prefix.



Comment at: test/CodeGenCXX/return.cpp:47
+  // CHECK-NOSTRICT-OPT-NEXT: zext
+  // CHECK-NOSTRICT-OPT-NEXT: ret i32
+}

Document what's going on in the tests please.



Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D26991: Hoist redundant load

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

There are no uses of `_LIBCPP_UNROLL_LOOPS` in LLVM (other than the ones in 
``.

Googling for `_LIBCPP_UNROLL_LOOPS` on github finds the ones in libc++, and no 
others.
I think I'll just take it out, and see what happens.


https://reviews.llvm.org/D26991



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

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/lib/CodeGen/CGBuilder.h:126
   // FIXME: these "default-aligned" APIs should be removed,
   // but I don't feel like fixing all the builtin code right now.
   llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val,

rjmccall wrote:
> Oh, please remove this comment, too, since you've now achieved it. :)
We still have CreateDefaultAlignedStore though.


https://reviews.llvm.org/D27157



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

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuilder.h:126
   // FIXME: these "default-aligned" APIs should be removed,
   // but I don't feel like fixing all the builtin code right now.
   llvm::StoreInst *CreateDefaultAlignedStore(llvm::Value *Val,

Oh, please remove this comment, too, since you've now achieved it. :)



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2195
 LoadInst *Load =
-Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true);
+Builder.CreateAlignedLoad(IntTy, IntToPtr, CharUnits::fromQuantity(4));
+Load->setVolatile(true);

pcc wrote:
> rjmccall wrote:
> > Why 4?
> __readfsdword is a Windows intrinsic which returns an unsigned long, which 
> always has size/alignment 4 on Windows.
> 
> But now that I think about it I suppose we can get here on other platforms 
> with MS extensions enabled, so I've changed this  to query the alignment.
Thanks.


https://reviews.llvm.org/D27157



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

[PATCH] D25928: [cfi] Enable cfi-icall on ARM and AArch64.

[Evgeniy Stepanov via Phabricator via cfe-commits]

eugenis closed this revision.
eugenis added a comment.

Committed in r286613


Repository:
  rL LLVM

https://reviews.llvm.org/D25928



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

[PATCH] D25869: [Driver] Add unit tests for Distro detection

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL288062: [Driver] Add unit tests for Distro detection 
(authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D25869?vs=78625=79444#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25869

Files:
  cfe/trunk/unittests/Driver/CMakeLists.txt
  cfe/trunk/unittests/Driver/DistroTest.cpp

Index: cfe/trunk/unittests/Driver/DistroTest.cpp
===
--- cfe/trunk/unittests/Driver/DistroTest.cpp
+++ cfe/trunk/unittests/Driver/DistroTest.cpp
@@ -0,0 +1,305 @@
+//===- unittests/Driver/DistroTest.cpp --- ToolChains tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Unit tests for Distro detection.
+//
+//===--===//
+
+#include "clang/Driver/Distro.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+using namespace clang;
+using namespace clang::driver;
+
+namespace {
+
+// The tests include all release-related files for each distribution
+// in the VFS, in order to make sure that earlier tests do not
+// accidentally result in incorrect distribution guess.
+
+TEST(DistroTest, DetectUbuntu) {
+  vfs::InMemoryFileSystem UbuntuTrustyFileSystem;
+  // Ubuntu uses Debian Sid version.
+  UbuntuTrustyFileSystem.addFile("/etc/debian_version", 0,
+  llvm::MemoryBuffer::getMemBuffer("jessie/sid\n"));
+  UbuntuTrustyFileSystem.addFile("/etc/lsb-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n"
+   "DISTRIB_RELEASE=14.04\n"
+   "DISTRIB_CODENAME=trusty\n"
+   "DISTRIB_DESCRIPTION=\"Ubuntu 14.04 LTS\"\n"));
+  UbuntuTrustyFileSystem.addFile("/etc/os-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n"
+   "VERSION=\"14.04, Trusty Tahr\"\n"
+   "ID=ubuntu\n"
+   "ID_LIKE=debian\n"
+   "PRETTY_NAME=\"Ubuntu 14.04 LTS\"\n"
+   "VERSION_ID=\"14.04\"\n"
+   "HOME_URL=\"http://www.ubuntu.com/\"\n;
+   "SUPPORT_URL=\"http://help.ubuntu.com/\"\n;
+   "BUG_REPORT_URL=\"http://bugs.launchpad.net/ubuntu/\"\n;));
+
+  Distro UbuntuTrusty{UbuntuTrustyFileSystem};
+  ASSERT_EQ(Distro(Distro::UbuntuTrusty), UbuntuTrusty);
+  ASSERT_TRUE(UbuntuTrusty.IsUbuntu());
+  ASSERT_FALSE(UbuntuTrusty.IsRedhat());
+  ASSERT_FALSE(UbuntuTrusty.IsOpenSUSE());
+  ASSERT_FALSE(UbuntuTrusty.IsDebian());
+
+  vfs::InMemoryFileSystem UbuntuYakketyFileSystem;
+  UbuntuYakketyFileSystem.addFile("/etc/debian_version", 0,
+  llvm::MemoryBuffer::getMemBuffer("stretch/sid\n"));
+  UbuntuYakketyFileSystem.addFile("/etc/lsb-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n"
+   "DISTRIB_RELEASE=16.10\n"
+   "DISTRIB_CODENAME=yakkety\n"
+   "DISTRIB_DESCRIPTION=\"Ubuntu 16.10\"\n"));
+  UbuntuYakketyFileSystem.addFile("/etc/os-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n"
+   "VERSION=\"16.10 (Yakkety Yak)\"\n"
+   "ID=ubuntu\n"
+   "ID_LIKE=debian\n"
+   "PRETTY_NAME=\"Ubuntu 16.10\"\n"
+   "VERSION_ID=\"16.10\"\n"
+   "HOME_URL=\"http://www.ubuntu.com/\"\n;
+   "SUPPORT_URL=\"http://help.ubuntu.com/\"\n;
+   "BUG_REPORT_URL=\"http://bugs.launchpad.net/ubuntu/\"\n;
+   "PRIVACY_POLICY_URL=\"http://www.ubuntu.com/legal/terms-and-policies/privacy-policy\"\n;
+   "VERSION_CODENAME=yakkety\n"
+   "UBUNTU_CODENAME=yakkety\n"));
+
+  Distro UbuntuYakkety{UbuntuYakketyFileSystem};
+  ASSERT_EQ(Distro(Distro::UbuntuYakkety), UbuntuYakkety);
+  ASSERT_TRUE(UbuntuYakkety.IsUbuntu());
+  ASSERT_FALSE(UbuntuYakkety.IsRedhat());
+  ASSERT_FALSE(UbuntuYakkety.IsOpenSUSE());
+  ASSERT_FALSE(UbuntuYakkety.IsDebian());
+}
+
+TEST(DistroTest, DetectRedhat) {
+  vfs::InMemoryFileSystem Fedora25FileSystem;
+  Fedora25FileSystem.addFile("/etc/system-release-cpe", 

[PATCH] D25949: [Driver] Refactor distro detection & classification as a separate API

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL288060: [Driver] Refactor distro detection & classification 
as a separate API (authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D25949?vs=78462=79442#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25949

Files:
  cfe/trunk/include/clang/Driver/Distro.h
  cfe/trunk/lib/Driver/CMakeLists.txt
  cfe/trunk/lib/Driver/Distro.cpp
  cfe/trunk/lib/Driver/ToolChains.cpp

Index: cfe/trunk/include/clang/Driver/Distro.h
===
--- cfe/trunk/include/clang/Driver/Distro.h
+++ cfe/trunk/include/clang/Driver/Distro.h
@@ -0,0 +1,122 @@
+//===--- Distro.h - Linux distribution detection support *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_DRIVER_DISTRO_H
+#define LLVM_CLANG_DRIVER_DISTRO_H
+
+#include "clang/Basic/VirtualFileSystem.h"
+
+namespace clang {
+namespace driver {
+
+/// Distro - Helper class for detecting and classifying Linux distributions.
+///
+/// This class encapsulates the clang Linux distribution detection mechanism
+/// as well as helper functions that match the specific (versioned) results
+/// into wider distribution classes.
+class Distro {
+public:
+  enum DistroType {
+// NB: Releases of a particular Linux distro should be kept together
+// in this enum, because some tests are done by integer comparison against
+// the first and last known member in the family, e.g. IsRedHat().
+ArchLinux,
+DebianLenny,
+DebianSqueeze,
+DebianWheezy,
+DebianJessie,
+DebianStretch,
+Exherbo,
+RHEL5,
+RHEL6,
+RHEL7,
+Fedora,
+OpenSUSE,
+UbuntuHardy,
+UbuntuIntrepid,
+UbuntuJaunty,
+UbuntuKarmic,
+UbuntuLucid,
+UbuntuMaverick,
+UbuntuNatty,
+UbuntuOneiric,
+UbuntuPrecise,
+UbuntuQuantal,
+UbuntuRaring,
+UbuntuSaucy,
+UbuntuTrusty,
+UbuntuUtopic,
+UbuntuVivid,
+UbuntuWily,
+UbuntuXenial,
+UbuntuYakkety,
+UbuntuZesty,
+UnknownDistro
+  };
+
+private:
+  /// The distribution, possibly with specific version.
+  DistroType DistroVal;
+
+public:
+  /// @name Constructors
+  /// @{
+
+  /// Default constructor leaves the distribution unknown.
+  Distro() : DistroVal() {}
+
+  /// Constructs a Distro type for specific distribution.
+  Distro(DistroType D) : DistroVal(D) {}
+
+  /// Detects the distribution using specified VFS.
+  explicit Distro(clang::vfs::FileSystem& VFS);
+
+  bool operator==(const Distro ) const {
+return DistroVal == Other.DistroVal;
+  }
+
+  bool operator!=(const Distro ) const {
+return DistroVal != Other.DistroVal;
+  }
+
+  bool operator>=(const Distro ) const {
+return DistroVal >= Other.DistroVal;
+  }
+
+  bool operator<=(const Distro ) const {
+return DistroVal <= Other.DistroVal;
+  }
+
+  /// @}
+  /// @name Convenience Predicates
+  /// @{
+
+  bool IsRedhat() const {
+return DistroVal == Fedora || (DistroVal >= RHEL5 && DistroVal <= RHEL7);
+  }
+
+  bool IsOpenSUSE() const {
+return DistroVal == OpenSUSE;
+  }
+
+  bool IsDebian() const {
+return DistroVal >= DebianLenny && DistroVal <= DebianStretch;
+  }
+
+  bool IsUbuntu() const {
+return DistroVal >= UbuntuHardy && DistroVal <= UbuntuZesty;
+  }
+ 
+  /// @}
+};
+
+} // end namespace driver
+} // end namespace clang
+
+#endif
Index: cfe/trunk/lib/Driver/CMakeLists.txt
===
--- cfe/trunk/lib/Driver/CMakeLists.txt
+++ cfe/trunk/lib/Driver/CMakeLists.txt
@@ -12,6 +12,7 @@
   Action.cpp
   Compilation.cpp
   CrossWindowsToolChain.cpp
+  Distro.cpp
   Driver.cpp
   DriverOptions.cpp
   Job.cpp
Index: cfe/trunk/lib/Driver/Distro.cpp
===
--- cfe/trunk/lib/Driver/Distro.cpp
+++ cfe/trunk/lib/Driver/Distro.cpp
@@ -0,0 +1,131 @@
+//===--- Distro.cpp - Linux distribution detection support --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Driver/Distro.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/MemoryBuffer.h"
+
+using namespace clang::driver;
+using namespace clang;
+
+static Distro::DistroType DetectDistro(vfs::FileSystem ) {
+  llvm::ErrorOr File =
+  VFS.getBufferForFile("/etc/lsb-release");
+  if (File) {
+

r288062 - [Driver] Add unit tests for Distro detection

[Michal Gorny via cfe-commits]

Author: mgorny
Date: Mon Nov 28 15:11:22 2016
New Revision: 288062

URL: http://llvm.org/viewvc/llvm-project?rev=288062=rev
Log:
[Driver] Add unit tests for Distro detection

Add a set of unit tests for the distro detection code. The tests use an
in-memory virtual filesystems resembling release files for various
distributions supported. All release files are provided (not only the
ones directly used) in order to guarantee that one of the rules will not
mistakenly recognize the distribution incorrectly due to the additional
files (e.g. Ubuntu as Debian).

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

Added:
cfe/trunk/unittests/Driver/DistroTest.cpp
Modified:
cfe/trunk/unittests/Driver/CMakeLists.txt

Modified: cfe/trunk/unittests/Driver/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/CMakeLists.txt?rev=288062=288061=288062=diff
==
--- cfe/trunk/unittests/Driver/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Driver/CMakeLists.txt Mon Nov 28 15:11:22 2016
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_unittest(ClangDriverTests
+  DistroTest.cpp
   ToolChainTest.cpp
   MultilibTest.cpp
   )

Added: cfe/trunk/unittests/Driver/DistroTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/DistroTest.cpp?rev=288062=auto
==
--- cfe/trunk/unittests/Driver/DistroTest.cpp (added)
+++ cfe/trunk/unittests/Driver/DistroTest.cpp Mon Nov 28 15:11:22 2016
@@ -0,0 +1,305 @@
+//===- unittests/Driver/DistroTest.cpp --- ToolChains tests 
---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Unit tests for Distro detection.
+//
+//===--===//
+
+#include "clang/Driver/Distro.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+using namespace clang;
+using namespace clang::driver;
+
+namespace {
+
+// The tests include all release-related files for each distribution
+// in the VFS, in order to make sure that earlier tests do not
+// accidentally result in incorrect distribution guess.
+
+TEST(DistroTest, DetectUbuntu) {
+  vfs::InMemoryFileSystem UbuntuTrustyFileSystem;
+  // Ubuntu uses Debian Sid version.
+  UbuntuTrustyFileSystem.addFile("/etc/debian_version", 0,
+  llvm::MemoryBuffer::getMemBuffer("jessie/sid\n"));
+  UbuntuTrustyFileSystem.addFile("/etc/lsb-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n"
+   "DISTRIB_RELEASE=14.04\n"
+   "DISTRIB_CODENAME=trusty\n"
+   "DISTRIB_DESCRIPTION=\"Ubuntu 14.04 
LTS\"\n"));
+  UbuntuTrustyFileSystem.addFile("/etc/os-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n"
+   "VERSION=\"14.04, Trusty Tahr\"\n"
+   "ID=ubuntu\n"
+   "ID_LIKE=debian\n"
+   "PRETTY_NAME=\"Ubuntu 14.04 LTS\"\n"
+   "VERSION_ID=\"14.04\"\n"
+   "HOME_URL=\"http://www.ubuntu.com/\"\n;
+   
"SUPPORT_URL=\"http://help.ubuntu.com/\"\n;
+   
"BUG_REPORT_URL=\"http://bugs.launchpad.net/ubuntu/\"\n;));
+
+  Distro UbuntuTrusty{UbuntuTrustyFileSystem};
+  ASSERT_EQ(Distro(Distro::UbuntuTrusty), UbuntuTrusty);
+  ASSERT_TRUE(UbuntuTrusty.IsUbuntu());
+  ASSERT_FALSE(UbuntuTrusty.IsRedhat());
+  ASSERT_FALSE(UbuntuTrusty.IsOpenSUSE());
+  ASSERT_FALSE(UbuntuTrusty.IsDebian());
+
+  vfs::InMemoryFileSystem UbuntuYakketyFileSystem;
+  UbuntuYakketyFileSystem.addFile("/etc/debian_version", 0,
+  llvm::MemoryBuffer::getMemBuffer("stretch/sid\n"));
+  UbuntuYakketyFileSystem.addFile("/etc/lsb-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("DISTRIB_ID=Ubuntu\n"
+   "DISTRIB_RELEASE=16.10\n"
+   "DISTRIB_CODENAME=yakkety\n"
+   "DISTRIB_DESCRIPTION=\"Ubuntu 
16.10\"\n"));
+  UbuntuYakketyFileSystem.addFile("/etc/os-release", 0,
+  llvm::MemoryBuffer::getMemBuffer("NAME=\"Ubuntu\"\n"
+   "VERSION=\"16.10 (Yakkety Yak)\"\n"
+   "ID=ubuntu\n"
+   "ID_LIKE=debian\n"
+   "PRETTY_NAME=\"Ubuntu 16.10\"\n"
+

r288061 - [Driver] Fix recognizing newer OpenSUSE versions

[Michal Gorny via cfe-commits]

Author: mgorny
Date: Mon Nov 28 15:11:18 2016
New Revision: 288061

URL: http://llvm.org/viewvc/llvm-project?rev=288061=rev
Log:
[Driver] Fix recognizing newer OpenSUSE versions

Fix recognizing newer OpenSUSE versions that combine the two version
components into 'VERSION = x.y'. The check was written against an older
version that kept those two split as VERSION and PATCHLEVEL.

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

Modified:
cfe/trunk/lib/Driver/Distro.cpp

Modified: cfe/trunk/lib/Driver/Distro.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Distro.cpp?rev=288061=288060=288061=diff
==
--- cfe/trunk/lib/Driver/Distro.cpp (original)
+++ cfe/trunk/lib/Driver/Distro.cpp Mon Nov 28 15:11:18 2016
@@ -108,11 +108,14 @@ static Distro::DistroType DetectDistro(v
   if (!Line.trim().startswith("VERSION"))
 continue;
   std::pair SplitLine = Line.split('=');
+  // Old versions have split VERSION and PATCHLEVEL
+  // Newer versions use VERSION = x.y
+  std::pair SplitVer = 
SplitLine.second.trim().split('.');
   int Version;
+
   // OpenSUSE/SLES 10 and older are not supported and not compatible
   // with our rules, so just treat them as Distro::UnknownDistro.
-  if (!SplitLine.second.trim().getAsInteger(10, Version) &&
-  Version > 10)
+  if (!SplitVer.first.getAsInteger(10, Version) && Version > 10)
 return Distro::OpenSUSE;
   return Distro::UnknownDistro;
 }


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

r288060 - [Driver] Refactor distro detection & classification as a separate API

[Michal Gorny via cfe-commits]

Author: mgorny
Date: Mon Nov 28 15:11:14 2016
New Revision: 288060

URL: http://llvm.org/viewvc/llvm-project?rev=288060=rev
Log:
[Driver] Refactor distro detection & classification as a separate API

Refactor the Distro enum along with helper functions into a full-fledged
Distro class, inspired by llvm::Triple, and make it a public API.
The new class wraps the enum with necessary comparison operators, adding
the convenience Is*() methods and a constructor performing
the detection. The public API is needed to run the unit tests (D25869).

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

Added:
cfe/trunk/include/clang/Driver/Distro.h
cfe/trunk/lib/Driver/Distro.cpp
Modified:
cfe/trunk/lib/Driver/CMakeLists.txt
cfe/trunk/lib/Driver/ToolChains.cpp

Added: cfe/trunk/include/clang/Driver/Distro.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Distro.h?rev=288060=auto
==
--- cfe/trunk/include/clang/Driver/Distro.h (added)
+++ cfe/trunk/include/clang/Driver/Distro.h Mon Nov 28 15:11:14 2016
@@ -0,0 +1,122 @@
+//===--- Distro.h - Linux distribution detection support *- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_DRIVER_DISTRO_H
+#define LLVM_CLANG_DRIVER_DISTRO_H
+
+#include "clang/Basic/VirtualFileSystem.h"
+
+namespace clang {
+namespace driver {
+
+/// Distro - Helper class for detecting and classifying Linux distributions.
+///
+/// This class encapsulates the clang Linux distribution detection mechanism
+/// as well as helper functions that match the specific (versioned) results
+/// into wider distribution classes.
+class Distro {
+public:
+  enum DistroType {
+// NB: Releases of a particular Linux distro should be kept together
+// in this enum, because some tests are done by integer comparison against
+// the first and last known member in the family, e.g. IsRedHat().
+ArchLinux,
+DebianLenny,
+DebianSqueeze,
+DebianWheezy,
+DebianJessie,
+DebianStretch,
+Exherbo,
+RHEL5,
+RHEL6,
+RHEL7,
+Fedora,
+OpenSUSE,
+UbuntuHardy,
+UbuntuIntrepid,
+UbuntuJaunty,
+UbuntuKarmic,
+UbuntuLucid,
+UbuntuMaverick,
+UbuntuNatty,
+UbuntuOneiric,
+UbuntuPrecise,
+UbuntuQuantal,
+UbuntuRaring,
+UbuntuSaucy,
+UbuntuTrusty,
+UbuntuUtopic,
+UbuntuVivid,
+UbuntuWily,
+UbuntuXenial,
+UbuntuYakkety,
+UbuntuZesty,
+UnknownDistro
+  };
+
+private:
+  /// The distribution, possibly with specific version.
+  DistroType DistroVal;
+
+public:
+  /// @name Constructors
+  /// @{
+
+  /// Default constructor leaves the distribution unknown.
+  Distro() : DistroVal() {}
+
+  /// Constructs a Distro type for specific distribution.
+  Distro(DistroType D) : DistroVal(D) {}
+
+  /// Detects the distribution using specified VFS.
+  explicit Distro(clang::vfs::FileSystem& VFS);
+
+  bool operator==(const Distro ) const {
+return DistroVal == Other.DistroVal;
+  }
+
+  bool operator!=(const Distro ) const {
+return DistroVal != Other.DistroVal;
+  }
+
+  bool operator>=(const Distro ) const {
+return DistroVal >= Other.DistroVal;
+  }
+
+  bool operator<=(const Distro ) const {
+return DistroVal <= Other.DistroVal;
+  }
+
+  /// @}
+  /// @name Convenience Predicates
+  /// @{
+
+  bool IsRedhat() const {
+return DistroVal == Fedora || (DistroVal >= RHEL5 && DistroVal <= RHEL7);
+  }
+
+  bool IsOpenSUSE() const {
+return DistroVal == OpenSUSE;
+  }
+
+  bool IsDebian() const {
+return DistroVal >= DebianLenny && DistroVal <= DebianStretch;
+  }
+
+  bool IsUbuntu() const {
+return DistroVal >= UbuntuHardy && DistroVal <= UbuntuZesty;
+  }
+ 
+  /// @}
+};
+
+} // end namespace driver
+} // end namespace clang
+
+#endif

Modified: cfe/trunk/lib/Driver/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/CMakeLists.txt?rev=288060=288059=288060=diff
==
--- cfe/trunk/lib/Driver/CMakeLists.txt (original)
+++ cfe/trunk/lib/Driver/CMakeLists.txt Mon Nov 28 15:11:14 2016
@@ -12,6 +12,7 @@ add_clang_library(clangDriver
   Action.cpp
   Compilation.cpp
   CrossWindowsToolChain.cpp
+  Distro.cpp
   Driver.cpp
   DriverOptions.cpp
   Job.cpp

Added: cfe/trunk/lib/Driver/Distro.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Distro.cpp?rev=288060=auto
==
--- cfe/trunk/lib/Driver/Distro.cpp (added)
+++ cfe/trunk/lib/Driver/Distro.cpp Mon Nov 28 15:11:14 2016
@@ -0,0 +1,131 @@
+//===--- Distro.cpp - Linux 

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: lib/Sema/AnalysisBasedWarnings.cpp:530
+  // Don't need to mark Objective-C methods or blocks since the undefined
+  // behaviour optimization isn't used for them.
+}

This seems like a trap waiting to spring on someone, unless there's a technical 
reason that methods and blocks cannot possibly use the same optimization paths 
as regular functions. ("Nobody's gotten around to implementing it yet" is the 
most obvious nontechnical reason for the current difference.) Either way, I'd 
expect this patch to include test cases for both methods and blocks, to verify 
that the behavior you expect is actually the behavior that happens. Basically, 
it ought to have a regression test targeting the regression that I'm predicting 
is going to spring on someone as soon as they implement optimizations for 
methods and blocks.

Also, one dumb question: what about C++ lambdas? are they FunctionDecls too? 
test case?



Comment at: test/CodeGenCXX/return.cpp:21
+  // CHECK-NOSTRICT-NEXT: load
+  // CHECK-NOSTRICT-NEXT: ret i32
+  // CHECK-NOSTRICT-NEXT: }

Can you explain why a load from an uninitialized stack location would be 
*better* than a trap and/or `unreachable`? IIUC this is basically what Mehdi is 
asking: i.e., can you explain the rationale for this patch, because I don't get 
it either. It *seems* like a strict regression in code quality.


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D26916: [ObjC] Avoid a @try/@finally/@autoreleasepool fixit when parsing an expression

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Alex, thanks for working on this




Comment at: lib/Parse/ParseObjc.cpp:2877
+if (GetLookAheadToken(1).is(tok::l_brace) &&
+ExprStatementTokLoc == AtLoc) {
   char ch = Tok.getIdentifierInfo()->getNameStart()[0];

Does this only triggers when `Res.isInvalid()` returns true in the first part 
of the patch? I wonder if it's also safe to allow  `ExprStatementTokLoc = 
AtLoc;` for every path or only when it fails.


Repository:
  rL LLVM

https://reviews.llvm.org/D26916



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

r288059 - [MS] Mangle a unique ID into all MS inline asm labels

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Nov 28 14:52:19 2016
New Revision: 288059

URL: http://llvm.org/viewvc/llvm-project?rev=288059=rev
Log:
[MS] Mangle a unique ID into all MS inline asm labels

This solves PR23715 in a way that is compatible with LTO.

MSVC supports jumping to source-level labels and between inline asm
blocks, but we don't.

Also revert the old solution, r255201, which was to mark these calls as
noduplicate.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=288059=288058=288059=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Nov 28 14:52:19 2016
@@ -788,9 +788,6 @@ public:
   /// \brief will hold 'respondsToSelector:'
   Selector RespondsToSelectorSel;
 
-  /// \brief counter for internal MS Asm label names.
-  unsigned MSAsmLabelNameCounter;
-
   /// A flag to remember whether the implicit forms of operator new and delete
   /// have been declared.
   bool GlobalNewDeleteDeclared;

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=288059=288058=288059=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Mon Nov 28 14:52:19 2016
@@ -2103,15 +2103,6 @@ void CodeGenFunction::EmitAsmStmt(const
   Result->addAttribute(llvm::AttributeSet::FunctionIndex,
llvm::Attribute::NoUnwind);
 
-  if (isa()) {
-// If the assembly contains any labels, mark the call noduplicate to 
prevent
-// defining the same ASM label twice (PR23715). This is pretty hacky, but 
it
-// works.
-if (AsmString.find("__MSASMLABEL_") != std::string::npos)
-  Result->addAttribute(llvm::AttributeSet::FunctionIndex,
-   llvm::Attribute::NoDuplicate);
-  }
-
   // Attach readnone and readonly attributes.
   if (!HasSideEffect) {
 if (ReadNone)

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=288059=288058=288059=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Mon Nov 28 14:52:19 2016
@@ -96,7 +96,6 @@ Sema::Sema(Preprocessor , ASTContext
 ValueWithBytesObjCTypeMethod(nullptr),
 NSArrayDecl(nullptr), ArrayWithObjectsMethod(nullptr),
 NSDictionaryDecl(nullptr), DictionaryWithObjectsMethod(nullptr),
-MSAsmLabelNameCounter(0),
 GlobalNewDeleteDeclared(false),
 TUKind(TUKind),
 NumSFINAEErrors(0),

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=288059=288058=288059=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Mon Nov 28 14:52:19 2016
@@ -753,14 +753,12 @@ LabelDecl *Sema::GetOrCreateMSAsmLabel(S
 // Create an internal name for the label.  The name should not be a valid 
mangled
 // name, and should be unique.  We use a dot to make the name an invalid 
mangled
 // name.
-OS << "__MSASMLABEL_." << MSAsmLabelNameCounter++ << "__";
-for (auto it = ExternalLabelName.begin(); it != ExternalLabelName.end();
- ++it) {
-  OS << *it;
-  if (*it == '$') {
-// We escape '$' in asm strings by replacing it with "$$"
+OS << "__MSASMLABEL_.{:uid}__";
+for (char C : ExternalLabelName) {
+  OS << C;
+  // We escape '$' in asm strings by replacing it with "$$"
+  if (C == '$')
 OS << '$';
-  }
 }
 Label->setMSAsmLabel(OS.str());
   }

Modified: cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c?rev=288059=288058=288059=diff
==
--- cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/mozilla-ms-inline-asm.c Mon Nov 28 14:52:19 2016
@@ -20,7 +20,7 @@ void invoke(void* that, unsigned methodI
 // CHECK: call void asm sideeffect inteldialect
 // CHECK: mov edx,dword ptr $1
 // CHECK: test edx,edx
-// CHECK: jz {{[^_]*}}__MSASMLABEL_.0__noparams
+// CHECK: jz {{[^_]*}}__MSASMLABEL_.{:uid}__noparams
 // ^ Can't use {{.*}} here because the matching is greedy.
 // CHECK: mov eax,edx
 // CHECK: shl eax,$$3
@@ -28,7 +28,7 @@ void 

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2195
 LoadInst *Load =
-Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true);
+Builder.CreateAlignedLoad(IntTy, IntToPtr, CharUnits::fromQuantity(4));
+Load->setVolatile(true);

rjmccall wrote:
> Why 4?
__readfsdword is a Windows intrinsic which returns an unsigned long, which 
always has size/alignment 4 on Windows.

But now that I think about it I suppose we can get here on other platforms with 
MS extensions enabled, so I've changed this  to query the alignment.


https://reviews.llvm.org/D27157



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

[PATCH] D27157: IRGen: Remove all uses of CreateDefaultAlignedLoad.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc updated this revision to Diff 79436.
pcc marked an inline comment as done.
pcc added a comment.

- Address review comments


https://reviews.llvm.org/D27157

Files:
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -3548,15 +3548,17 @@
 llvm::Value *RegHiAddr = TyLo->isFPOrFPVectorTy() ? GPAddr : FPAddr;
 
 // Copy the first element.
-llvm::Value *V =
-  CGF.Builder.CreateDefaultAlignedLoad(
-   CGF.Builder.CreateBitCast(RegLoAddr, PTyLo));
+// FIXME: Our choice of alignment here and below is probably pessimistic.
+llvm::Value *V = CGF.Builder.CreateAlignedLoad(
+TyLo, CGF.Builder.CreateBitCast(RegLoAddr, PTyLo),
+CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(TyLo)));
 CGF.Builder.CreateStore(V,
 CGF.Builder.CreateStructGEP(Tmp, 0, CharUnits::Zero()));
 
 // Copy the second element.
-V = CGF.Builder.CreateDefaultAlignedLoad(
-   CGF.Builder.CreateBitCast(RegHiAddr, PTyHi));
+V = CGF.Builder.CreateAlignedLoad(
+TyHi, CGF.Builder.CreateBitCast(RegHiAddr, PTyHi),
+CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(TyHi)));
 CharUnits Offset = CharUnits::fromQuantity(
getDataLayout().getStructLayout(ST)->getElementOffset(1));
 CGF.Builder.CreateStore(V, CGF.Builder.CreateStructGEP(Tmp, 1, Offset));
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2890,9 +2890,11 @@
 if (RuntimeVersion < 10 ||
 CGF.CGM.getTarget().getTriple().isKnownWindowsMSVCEnvironment())
   return CGF.Builder.CreateZExtOrBitCast(
-  CGF.Builder.CreateDefaultAlignedLoad(CGF.Builder.CreateAlignedLoad(
-  ObjCIvarOffsetVariable(Interface, Ivar),
-  CGF.getPointerAlign(), "ivar")),
+  CGF.Builder.CreateAlignedLoad(
+  Int32Ty, CGF.Builder.CreateAlignedLoad(
+   ObjCIvarOffsetVariable(Interface, Ivar),
+   CGF.getPointerAlign(), "ivar"),
+  CharUnits::fromQuantity(4)),
   PtrDiffTy);
 std::string name = "__objc_ivar_offset_value_" +
   Interface->getNameAsString() +"." + Ivar->getNameAsString();
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2884,13 +2884,13 @@
   // FIXME: Generate IR in one pass, rather than going back and fixing up these
   // placeholders.
   llvm::Type *IRTy = CGF.ConvertTypeForMem(Ty);
-  llvm::Value *Placeholder =
-llvm::UndefValue::get(IRTy->getPointerTo()->getPointerTo());
-  Placeholder = CGF.Builder.CreateDefaultAlignedLoad(Placeholder);
+  llvm::Type *IRPtrTy = IRTy->getPointerTo();
+  llvm::Value *Placeholder = llvm::UndefValue::get(IRPtrTy->getPointerTo());
 
   // FIXME: When we generate this IR in one pass, we shouldn't need
   // this win32-specific alignment hack.
   CharUnits Align = CharUnits::fromQuantity(4);
+  Placeholder = CGF.Builder.CreateAlignedLoad(IRPtrTy, Placeholder, Align);
 
   return AggValueSlot::forAddr(Address(Placeholder, Align),
Ty.getQualifiers(),
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2191,8 +2191,9 @@
 Value *IntToPtr =
   Builder.CreateIntToPtr(EmitScalarExpr(E->getArg(0)),
  llvm::PointerType::get(IntTy, 257));
-LoadInst *Load =
-Builder.CreateDefaultAlignedLoad(IntToPtr, /*isVolatile=*/true);
+LoadInst *Load = Builder.CreateAlignedLoad(
+IntTy, IntToPtr, getContext().getTypeAlignInChars(E->getType()));
+Load->setVolatile(true);
 return RValue::get(Load);
   }
 
@@ -5440,9 +5441,11 @@
   switch (BuiltinID) {
   default: break;
   case NEON::BI__builtin_neon_vldrq_p128: {
-llvm::Type *Int128PTy = llvm::Type::getIntNPtrTy(getLLVMContext(), 128);
+llvm::Type *Int128Ty = llvm::Type::getIntNTy(getLLVMContext(), 128);
+llvm::Type *Int128PTy = llvm::PointerType::get(Int128Ty, 0);
 Value *Ptr = Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int128PTy);
-return Builder.CreateDefaultAlignedLoad(Ptr);
+return Builder.CreateAlignedLoad(Int128Ty, Ptr,
+ CharUnits::fromQuantity(16));
   }
   case NEON::BI__builtin_neon_vstrq_p128: {
 llvm::Type *Int128PTy = 

[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added inline comments.



Comment at: lib/Basic/TargetInfo.cpp:229
   switch (BitWidth) {
-  case 96:
+  case 80:
 if (() == ::APFloat::x87DoubleExtended)

The change makes sense but I believe there's some historical reason why this is 
96 instead of 80? What problem have you found in practice? Do you have a 
testcase or unittest that exercise the issue (and that could be added to the 
patch)?


https://reviews.llvm.org/D26955



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

[PATCH] D24991: Inline hot functions in libcxx shared_ptr implementation.

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

@mclow.lists could you please have a last look at this patch: the change is for 
a performance improvement (20% uplift on a proprietary benchmark), and all the 
issues mentioned in the review have been addressed.
The existing synthetic benchmark shows an overall improvement:

  master:
  Benchmark  Time   CPU Iterations
  
  BM_SharedPtrCreateDestroy 54 ns 54 ns   12388755
  BM_SharedPtrIncDecRef 37 ns 37 ns   19021739
  BM_WeakPtrIncDecRef   38 ns 38 ns   18421053
   
  master + patch:
  Benchmark  Time   CPU Iterations
  
  BM_SharedPtrCreateDestroy 44 ns 44 ns   14730639
  BM_SharedPtrIncDecRef 18 ns 18 ns   3889
  BM_WeakPtrIncDecRef   30 ns 30 ns   23648649


https://reviews.llvm.org/D24991



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

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D27163#606744, @arphaman wrote:

> In https://reviews.llvm.org/D27163#606695, @mehdi_amini wrote:
>
> > What is the justification for a platform specific default change here?
>
>
> The flag itself is platform agnostic, however, the default value is different 
> on Darwin (-fno-strict-return). The reason for this difference is because of 
> how I interpreted the internal discussion for this issue: it seems that some 
> of our internal builds had problems with this flag, so to me it seemed that 
> people would've wanted this specific difference upstream.


I was not involved in the internal discussion, and the pre-existing internal 
arguments should be repeated here when needed to drive the patch.

I find dubious that the compiler wouldn't have specific handling for undefined 
behavior on a specific platform, without a strong argument that justify it 
(like a platform with a different hardware memory model making it more 
sensitive, etc.). In the current case, it seems like a "vendor specific choice" 
of being more conservative rather than something attached to the platform 
itself, so I'm not sure it makes sense to hard-code this upstream.

Do we have past records of doing this?


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D27068: Improve string::find

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

The numbers are from an x86_64-linux machine: Intel(R) Core(TM) i7-4790K CPU @ 
4.00GHz
Overall the patch is a win on the synthetic benchmark.
We have also seen this in a proprietary benchmark where it accounted for about 
10% speedup.


https://reviews.llvm.org/D27068



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

[PATCH] D27068: Improve string::find

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D27068#606757, @mclow.lists wrote:

> Definitely want to see numbers.




  master:
  Benchmark Time   CPU Iterations
  ---
  BM_StringFindPhase1/104 ns  4 ns  161568945
  BM_StringFindPhase1/64   28 ns 28 ns   24937005
  BM_StringFindPhase1/512 132 ns132 ns5321432
  BM_StringFindPhase1/4k  956 ns956 ns 732038
  BM_StringFindPhase1/32k7622 ns   7621 ns  92121
  BM_StringFindPhase1/128k  30485 ns  30483 ns  22938
  BM_StringFindPhase2/1 4 ns  4 ns  191884173
  BM_StringFindPhase2/8 7 ns  7 ns  105129099
  BM_StringFindPhase2/64   34 ns 34 ns   20582485
  BM_StringFindPhase2/512 187 ns187 ns3736654
  BM_StringFindPhase2/4k 1415 ns   1414 ns 495342
  BM_StringFindPhase2/32k   11221 ns  11220 ns  62393
  BM_StringFindPhase2/128k  44952 ns  44949 ns  15595
  
  master + patch:
  Benchmark Time   CPU Iterations
  ---
  BM_StringFindPhase1/103 ns  3 ns  204725158
  BM_StringFindPhase1/645 ns  5 ns  146268957
  BM_StringFindPhase1/512  12 ns 12 ns   60176557
  BM_StringFindPhase1/4k   67 ns 67 ns   10508533
  BM_StringFindPhase1/32k 503 ns503 ns100
  BM_StringFindPhase1/128k   2396 ns   2396 ns 292701
  BM_StringFindPhase2/1 6 ns  6 ns  127378641
  BM_StringFindPhase2/8 6 ns  6 ns  117407388
  BM_StringFindPhase2/647 ns  7 ns   95998016
  BM_StringFindPhase2/512  18 ns 18 ns   38135928
  BM_StringFindPhase2/4k   83 ns 83 ns8452337
  BM_StringFindPhase2/32k 762 ns762 ns 925744
  BM_StringFindPhase2/128k   3255 ns   3255 ns 215141


https://reviews.llvm.org/D27068



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

[PATCH] D26991: Hoist redundant load

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

In https://reviews.llvm.org/D26991#606764, @mclow.lists wrote:

> /me wonders what the perf difference when `_LIBCPP_UNROLL_LOOPS` is defined 
> or  not.
>
> I think this (`_LIBCPP_UNROLL_LOOPS`) falls squarely into Chandler's request 
> that we complain to him when the compiler generates sub-optimal code, instead 
> of doing things like manually unrolling loops.


I think we should remove all the code in the ifdef: it looks to me like this 
code was left in from a previous "experiment", as it never gets executed unless 
one compiles the code with -D_LIBCPP_UNROLL_LOOPS, which seems wrong.  Let's 
push this cleanup in a separate commit.


https://reviews.llvm.org/D26991



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

Re: Upgrade and fix clang-format-vs

[Hans Wennborg via cfe-commits]

On Mon, Nov 28, 2016 at 11:11 AM, Antonio Maiorano  wrote:
>> It's built with the script in utils/release/build_llvm_package.bat
> which I run manually on my machine once every few weeks.
>
> Okay, that's good news. So the simplest path to success would be to require
> the user to either pass the path to CMake via an arg like -DNUGET_EXE_PATH,
> or if it's not defined, to assume it's already in PATH. This is the most
> future-proof solution as it will work with future versions of VS (2017 RC
> just came out).
>
> I can still look into whether a vsix built with VS 2015 references will
> continue to work in older versions of VS, but even if this works, I feel
> like it's a temporary solution at best. There are other advantages to using
> NuGet here: it would allow us to more easily pin/upgrade which assemblies we
> want to use over time.
>
> If you're okay with it, I'll make the changes necessary to use
> -DNUGET_EXE_PATH, if defined, otherwise assume it's on PATH. This should be
> a simple change at this point.

That sounds good to me. There are already a bunch of prerequisites for
building the plugin, so adding this one doesn't seem unreasonable.
Especially since it seems it will simplify things to the point that
they might even work elsewhere than my own machine :-)


> On Mon, 28 Nov 2016 at 13:59 Hans Wennborg  wrote:
>>
>> On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano 
>> wrote:
>> > Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX
>> > to
>> > keep working in older versions of VS.
>> >
>> > Still waiting on an answer to this question:
>> >
>> >> In either case, though, I must ask: how is the offical vsix that's
>> >> available on http://llvm.org/builds/ get built? Is it part of an
>> >> automated
>> >> Clang build, or is it built and uploaded manually? If it's automated,
>> >> then
>> >> having to download and point to nuget.exe won't work.
>>
>> It's built with the script in utils/release/build_llvm_package.bat
>> which I run manually on my machine once every few weeks.
>>
>>
>> > On Mon, 28 Nov 2016 at 13:04 Hans Wennborg  wrote:
>> >>
>> >> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano 
>> >> wrote:
>> >> > Ah, no, that's not what I meant. The required referenced assemblies
>> >> > are
>> >> > versions that are normally installed with VS 2010.
>> >> >
>> >> > The first time I worked on this, I had upgraded the referenced
>> >> > assemblies to
>> >> > the ones that ship with VS 2015, but then there was question of
>> >> > whether
>> >> > or
>> >> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not
>> >> > sure
>> >> > if
>> >> > it would work, but I guess I can try to figure that out.
>> >>
>> >> Let me know if you figure this one out. It sounds like it would
>> >> simplify things a lot.
>> >>
>> >> > In any case, what I discovered is that the "right" way to do things
>> >> > to
>> >> > make
>> >> > sure your extension compiles in future versions of VS is to use NuGet
>> >> > to
>> >> > automatically pull in the required assemblies, or to check them in
>> >> > and
>> >> > reference them directly. The former would be better except for the
>> >> > problem
>> >> > of CLI builds as I described in my earlier email.
>> >> >
>> >> >
>> >> >
>> >> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner 
>> >> > wrote:
>> >> >>
>> >> >> Sorry, i think I misunderstood the original option 1. I interpreted
>> >> >> it
>> >> >> as
>> >> >> just committing changes to the vsix manifest to reference a specific
>> >> >> version
>> >> >> of the assembly which we assume to be present since it should be
>> >> >> automatically installed with vs 2015. Is this not possible? Can't we
>> >> >> just
>> >> >> point the manifest to the version installed with vs?
>> >> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano
>> >> >> 
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi again,
>> >> >>>
>> >> >>> I've made the changes so that the required assemblies are
>> >> >>> committed,
>> >> >>> so
>> >> >>> now we can build the clang-format-vsix with just VS 2015. Since the
>> >> >>> patch
>> >> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if
>> >> >>> you'd
>> >> >>> rather I attach it, let me know):
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>> >> >>>
>> >> >>> Note that it's a git patch set, for which I followed the
>> >> >>> instructions
>> >> >>> here.
>> >> >>>
>> >> >>> Thanks.
>> >> >>>
>> >> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano 
>> >> >>> wrote:
>> >> 
>> >>  Okay, that's fine, I'll go for that and if all looks good, will
>> >>  attach a
>> >>  patch.
>> >> 
>> >>  Thanks.
>> >> 
>> >>  On Thu, 24 Nov 2016 at 15:09 Zachary Turner 

[PATCH] D27167: [libc++] Support multibyte decimal_point and thousands_sep

[Eric van Gyzen via Phabricator via cfe-commits]

vangyzen added a comment.

A unit test change in https://reviews.llvm.org/D26979 found this and will 
continue to test it (on some OSes).


https://reviews.llvm.org/D27167



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

[PATCH] D26612: Protect std::string tests under libcpp-no-exceptions

[Marshall Clow via Phabricator via cfe-commits]

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

LGTM. Thanks.


https://reviews.llvm.org/D26612



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

[PATCH] D26611: Protect test for dynarray under libcpp-no-exceptions

[Marshall Clow via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D26611



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

[PATCH] D27095: Protect std::array tests under noexceptions

[Marshall Clow via Phabricator via cfe-commits]

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

Other than the missing `assert`s, (which are not your fault, but I would 
appreciate you fixing) this LGTM.




Comment at: test/std/containers/sequences/array/at.pass.cpp:43
+#ifndef TEST_HAS_NO_EXCEPTIONS
 try { (void) c.at(3); }
 catch (const std::out_of_range &) {}

we should really have an `assert(false);` after the call to `at` - to make sure 
that it actually throws.

Here and below.



https://reviews.llvm.org/D27095



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

[PATCH] D27093: Protect std::{, unordered_}map tests under noexceptions

[Marshall Clow via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D27093



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

Re: Upgrade and fix clang-format-vs

[Hans Wennborg via cfe-commits]

On Mon, Nov 28, 2016 at 10:46 AM, Antonio Maiorano  wrote:
> Okay, I'll see if upgrading to the 2015 assemblies would allow the VSIX to
> keep working in older versions of VS.
>
> Still waiting on an answer to this question:
>
>> In either case, though, I must ask: how is the offical vsix that's
>> available on http://llvm.org/builds/ get built? Is it part of an automated
>> Clang build, or is it built and uploaded manually? If it's automated, then
>> having to download and point to nuget.exe won't work.

It's built with the script in utils/release/build_llvm_package.bat
which I run manually on my machine once every few weeks.


> On Mon, 28 Nov 2016 at 13:04 Hans Wennborg  wrote:
>>
>> On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano 
>> wrote:
>> > Ah, no, that's not what I meant. The required referenced assemblies are
>> > versions that are normally installed with VS 2010.
>> >
>> > The first time I worked on this, I had upgraded the referenced
>> > assemblies to
>> > the ones that ship with VS 2015, but then there was question of whether
>> > or
>> > not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure
>> > if
>> > it would work, but I guess I can try to figure that out.
>>
>> Let me know if you figure this one out. It sounds like it would
>> simplify things a lot.
>>
>> > In any case, what I discovered is that the "right" way to do things to
>> > make
>> > sure your extension compiles in future versions of VS is to use NuGet to
>> > automatically pull in the required assemblies, or to check them in and
>> > reference them directly. The former would be better except for the
>> > problem
>> > of CLI builds as I described in my earlier email.
>> >
>> >
>> >
>> > On Fri, 25 Nov 2016 at 21:47 Zachary Turner  wrote:
>> >>
>> >> Sorry, i think I misunderstood the original option 1. I interpreted it
>> >> as
>> >> just committing changes to the vsix manifest to reference a specific
>> >> version
>> >> of the assembly which we assume to be present since it should be
>> >> automatically installed with vs 2015. Is this not possible? Can't we
>> >> just
>> >> point the manifest to the version installed with vs?
>> >> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
>> >> wrote:
>> >>>
>> >>> Hi again,
>> >>>
>> >>> I've made the changes so that the required assemblies are committed,
>> >>> so
>> >>> now we can build the clang-format-vsix with just VS 2015. Since the
>> >>> patch
>> >>> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
>> >>> rather I attach it, let me know):
>> >>>
>> >>>
>> >>>
>> >>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>> >>>
>> >>> Note that it's a git patch set, for which I followed the instructions
>> >>> here.
>> >>>
>> >>> Thanks.
>> >>>
>> >>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano 
>> >>> wrote:
>> 
>>  Okay, that's fine, I'll go for that and if all looks good, will
>>  attach a
>>  patch.
>> 
>>  Thanks.
>> 
>>  On Thu, 24 Nov 2016 at 15:09 Zachary Turner 
>>  wrote:
>> >
>> > I would use the first solution. We lock ourselves to specific
>> > versions
>> > of vs, so i think it's fine to do the same with the assemblies and
>> > deal with
>> > it only when we upgrade
>> > On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano
>> > 
>> > wrote:
>> >>
>> >> Hi Hans,
>> >>
>> >> I saw that on September 15th, you checked in a change: clang-format
>> >> VS
>> >> plugin: upgrade the project files to VS2015.
>> >>
>> >> When I open the latest version of ClangFormat.sln on a machine that
>> >> has only VS 2015, it doesn't build. The reason is that some of the
>> >> referenced assemblies are from VS 2010.
>> >>
>> >>  > >> Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0,
>> >> Culture=neutral,
>> >> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>> >>
>> >> What happens is that when building, these specific assemblies are
>> >> not
>> >> found. I suspect you have VS 2010 installed on your machine, which
>> >> is why
>> >> you don't get these build errors.
>> >>
>> >> The recommended way to handle this is to make use of NuGet to have
>> >> it
>> >> automatically download the required assemblies. I have made the
>> >> changes
>> >> locally to get this working, and it works great when building
>> >> ClangFormat.sln from within Visual Studio; however, building from
>> >> the CLI
>> >> doesn't work out of the box because you must explicitly run
>> >> 'nuget.exe
>> >> restore ClangFormat.sln' before running msbuild (or devenv.exe in
>> >> our case).
>> >> The problem is that 

[PATCH] D27166: [clang-tidy] Enhance modernize-use-auto to templated function casts

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

It'll be worth to mention enhancement in Release Notes.


https://reviews.llvm.org/D27166



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

[PATCH] D27096: Protect locale tests under noexceptions

[Marshall Clow via Phabricator via cfe-commits]

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

This LGTM.


https://reviews.llvm.org/D27096



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

[PATCH] D26896: [libcxx] Make constexpr char_traits and char_traits

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/__string:213
 
-static inline int compare(const char_type* __s1, const char_type* __s2, 
size_t __n) _NOEXCEPT
-{return __n == 0 ? 0 : memcmp(__s1, __s2, __n);}
-static inline size_t length(const char_type* __s)  _NOEXCEPT {return 
strlen(__s);}
-static inline const char_type* find(const char_type* __s, size_t __n, 
const char_type& __a) _NOEXCEPT
-{return __n == 0 ? NULL : (const char_type*) memchr(__s, 
to_int_type(__a), __n);}
+#if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_CXX14_CONSTEXPR)
+static inline constexpr int

AntonBikineev wrote:
> EricWF wrote:
> > wow. This is #ifdef hell. Please find a way to do it with less (or 
> > hopefully no) conditional compilation blocks.
> yep, this is generic hell. I want to cover as many cases as possible, i.e. 
> combinations of (is_constexpr x has_builtin_xxx) for every function. I'm open 
> to suggestions
How about (for compare, say) you just forget about `memcmp`.  Either call 
`__builtin_memcmp` if it is available, or use a hand-rolled loop.

Note: gcc has had `__builtin_memcmp` since at least 4.8. (and it is constexpr)

And just mark the function with `_LIBCPP_CONSTEXPR_AFTER_CXX14`.


https://reviews.llvm.org/D26896



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

[PATCH] D26991: Hoist redundant load

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

__search is the only place where `_LIBCPP_UNROLL_LOOPS` is currently used.


https://reviews.llvm.org/D26991



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

[PATCH] D26991: Hoist redundant load

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

/me wonders what the perf difference when `_LIBCPP_UNROLL_LOOPS` is defined or  
not.

I think this (`_LIBCPP_UNROLL_LOOPS`) falls squarely into Chandler's request 
that we complain to him when the compiler generates sub-optimal code, instead 
of doing things like manually unrolling loops.


https://reviews.llvm.org/D26991



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

[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386

[Evgeniy Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

I think this is the right move together with https://reviews.llvm.org/D26796.
Android build system will need to support both names for some time, depending 
on the toolchain version - looks doable. Technically, it can stick with the old 
name forever.


https://reviews.llvm.org/D26764



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

[PATCH] D27068: Improve string::find

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Definitely want to see numbers.


https://reviews.llvm.org/D27068



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

[PATCH] D26137: [clang-tidy] Add check name to YAML export

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

I'll try to get back to this code review soon. Sorry for the delay.


Repository:
  rL LLVM

https://reviews.llvm.org/D26137



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

[PATCH] D27140: Allow clang to write compilation database records

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg removed rL LLVM as the repository for this revision.
joerg updated this revision to Diff 79423.
joerg added a comment.

Use llvm::yaml::escape.


https://reviews.llvm.org/D27140

Files:
  include/clang/Driver/Options.td
  lib/Driver/Tools.cpp

Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -266,6 +266,8 @@
 MetaVarName<"">;
 def MG : Flag<["-"], "MG">, Group, Flags<[CC1Option]>,
 HelpText<"Add missing headers to depfile">;
+def MJ : JoinedOrSeparate<["-"], "MJ">, Group,
+HelpText<"Write a compilation database entry per input">;
 def MP : Flag<["-"], "MP">, Group, Flags<[CC1Option]>,
 HelpText<"Create phony target for each dependency (other than main file)">;
 def MQ : JoinedOrSeparate<["-"], "MQ">, Group, Flags<[CC1Option]>,
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/Support/YAMLParser.h"
 
 #ifdef LLVM_ON_UNIX
 #include  // For getuid().
@@ -4002,6 +4003,64 @@
 CmdArgs.push_back("-KPIC");
 }
 
+static void QuoteJSONString(llvm::raw_fd_ostream , StringRef Str) {
+  Stream << "\"";
+  Stream << llvm::yaml::escape(Str);
+  Stream << "\"";
+}
+
+static void DumpCompilationDatabase(const Driver , StringRef Filename, StringRef Target, const InputInfo ,
+const InputInfo , const ArgList ) {
+  std::error_code EC;
+  llvm::raw_fd_ostream File(Filename, EC, llvm::sys::fs::F_Text | llvm::sys::fs::F_Append);
+  if (EC) {
+//errs() << "Failed to open " << Filename << ": " << EC.message() << "\n" ;
+return;
+  }
+  SmallString<128> Buf;
+  if (llvm::sys::fs::current_path(Buf))
+Buf = ".";
+  File << "{ \"directory\": ";
+  QuoteJSONString(File, Buf);
+  File << ", \"file\": ";
+  QuoteJSONString(File, Input.getFilename());
+  File << ", \"output\": ";
+  QuoteJSONString(File, Output.getFilename());
+
+  File << ", \"arguments\": [";
+  QuoteJSONString(File, D.ClangExecutable);
+  File << ", ";
+  Buf = "-x";
+  Buf += types::getTypeName(Input.getType());
+  QuoteJSONString(File, Buf);
+  File << ", ";
+  QuoteJSONString(File, Input.getFilename());
+  for (auto : Args) {
+auto  = A->getOption();
+// Skip language selection, which is positional.
+if (O.getID() == options::OPT_x)
+  continue;
+// Skip writing dependency output and the compiliation database itself.
+if (O.getGroup().isValid() && O.getGroup().getID() == options::OPT_M_Group)
+  continue;
+// Skip inputs.
+if (O.getKind() == Option::InputClass)
+  continue;
+// All other arguments are quoted and appended.
+ArgStringList ASL;
+A->render(Args, ASL);
+for (auto : ASL) {
+  File << ", ";
+  QuoteJSONString(File, it);
+}
+  }
+  File << ", ";
+  Buf = "--target=";
+  Buf += Target;
+  QuoteJSONString(File, Buf);
+  File << "]},\n";
+}
+
 void Clang::ConstructJob(Compilation , const JobAction ,
  const InputInfo , const InputInfoList ,
  const ArgList , const char *LinkingOutput) const {
@@ -4046,6 +4105,10 @@
   CmdArgs.push_back("-triple");
   CmdArgs.push_back(Args.MakeArgString(TripleStr));
 
+  if (const Arg *MJ = Args.getLastArg(options::OPT_MJ))
+DumpCompilationDatabase(C.getDriver(), MJ->getValue(), TripleStr, Output, Input, Args);
+  Args.ClaimAllArgs(options::OPT_MJ);
+
   if (IsCuda) {
 // We have to pass the triple of the host if compiling for a CUDA device and
 // vice-versa.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D27163#606695, @mehdi_amini wrote:

> What is the justification for a platform specific default change here?


The flag itself is platform agnostic, however, the default value is different 
on Darwin (-fno-strict-return). The reason for this difference is because of 
how I interpreted the internal discussion for this issue: it seems that some of 
our internal builds had problems with this flag, so to me it seemed that people 
would've wanted this specific difference upstream. I might be wrong though and 
this might be not even the best approach, so let me know if you think there's a 
better solution.


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D27068: Improve string::find

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

Please also post the performance numbers from the added benchmarks with and 
without the change to the lib.




Comment at: libcxx/benchmarks/string.bench.cpp:12
+// until the end of s1.
+static void BM_StringFindPhase1(benchmark::State& state) {
+  // Benchmark following the length of s1.

Let's call this benchmark "BM_StringFindNoMatch"



Comment at: libcxx/benchmarks/string.bench.cpp:21
+
+// Benchmark the __phase2 part of string search: we want the strings s1 and s2
+// to match from the first char in s1.

Please reword to remove the reference to __phase2.



Comment at: libcxx/benchmarks/string.bench.cpp:23
+// to match from the first char in s1.
+static void BM_StringFindPhase2(benchmark::State& state) {
+  std::string s1(MAX_STRING_LEN, '-');

Let's call this benchmark "BM_StringFindAllMatch"


https://reviews.llvm.org/D27068



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

[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

In https://reviews.llvm.org/D26764#606638, @beanz wrote:

> Looping in @eugenis, who added i686 support in r218761.
>
> eugenis, since i686 is identical to i386 generating it as a separate target 
> is undesirable. Can you help advise as to how we can better meet your needs 
> and not duplicate an effectively identical target?


I suspect https://reviews.llvm.org/D26796 attempts to solve the same problem.


https://reviews.llvm.org/D26764



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

[PATCH] D26991: Hoist redundant load

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.

Let's also add a testcase and show the performance improvement.


https://reviews.llvm.org/D26991



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

Re: Upgrade and fix clang-format-vs

[Hans Wennborg via cfe-commits]

On Fri, Nov 25, 2016 at 6:58 PM, Antonio Maiorano  wrote:
> Ah, no, that's not what I meant. The required referenced assemblies are
> versions that are normally installed with VS 2010.
>
> The first time I worked on this, I had upgraded the referenced assemblies to
> the ones that ship with VS 2015, but then there was question of whether or
> not the VSIX would continue to work with VS 2010/2012/2013. I'm not sure if
> it would work, but I guess I can try to figure that out.

Let me know if you figure this one out. It sounds like it would
simplify things a lot.

> In any case, what I discovered is that the "right" way to do things to make
> sure your extension compiles in future versions of VS is to use NuGet to
> automatically pull in the required assemblies, or to check them in and
> reference them directly. The former would be better except for the problem
> of CLI builds as I described in my earlier email.
>
>
>
> On Fri, 25 Nov 2016 at 21:47 Zachary Turner  wrote:
>>
>> Sorry, i think I misunderstood the original option 1. I interpreted it as
>> just committing changes to the vsix manifest to reference a specific version
>> of the assembly which we assume to be present since it should be
>> automatically installed with vs 2015. Is this not possible? Can't we just
>> point the manifest to the version installed with vs?
>> On Fri, Nov 25, 2016 at 6:20 PM Antonio Maiorano 
>> wrote:
>>>
>>> Hi again,
>>>
>>> I've made the changes so that the required assemblies are committed, so
>>> now we can build the clang-format-vsix with just VS 2015. Since the patch
>>> set is around 9 mb, I'm providing a link to it on my Dropbox (if you'd
>>> rather I attach it, let me know):
>>>
>>>
>>> https://dl.dropboxusercontent.com/u/10504225/llvm-patches/0001-Fix-VS2015-build-of-clang-format-vsix-by-committing-.patch
>>>
>>> Note that it's a git patch set, for which I followed the instructions
>>> here.
>>>
>>> Thanks.
>>>
>>> On Thu, 24 Nov 2016 at 15:45 Antonio Maiorano 
>>> wrote:

 Okay, that's fine, I'll go for that and if all looks good, will attach a
 patch.

 Thanks.

 On Thu, 24 Nov 2016 at 15:09 Zachary Turner  wrote:
>
> I would use the first solution. We lock ourselves to specific versions
> of vs, so i think it's fine to do the same with the assemblies and deal 
> with
> it only when we upgrade
> On Thu, Nov 24, 2016 at 11:29 AM Antonio Maiorano 
> wrote:
>>
>> Hi Hans,
>>
>> I saw that on September 15th, you checked in a change: clang-format VS
>> plugin: upgrade the project files to VS2015.
>>
>> When I open the latest version of ClangFormat.sln on a machine that
>> has only VS 2015, it doesn't build. The reason is that some of the
>> referenced assemblies are from VS 2010.
>>
>>  > Include="Microsoft.VisualStudio.Editor, Version=10.0.0.0, 
>> Culture=neutral,
>> PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
>>
>> What happens is that when building, these specific assemblies are not
>> found. I suspect you have VS 2010 installed on your machine, which is why
>> you don't get these build errors.
>>
>> The recommended way to handle this is to make use of NuGet to have it
>> automatically download the required assemblies. I have made the changes
>> locally to get this working, and it works great when building
>> ClangFormat.sln from within Visual Studio; however, building from the CLI
>> doesn't work out of the box because you must explicitly run 'nuget.exe
>> restore ClangFormat.sln' before running msbuild (or devenv.exe in our 
>> case).
>> The problem is that nuget.exe isn't something that's easily 
>> found/accessible
>> on Windows, even once installed as an extension in VS. This is a known
>> problem and the current best solution requires downloading and making
>> nuget.exe available in PATH.
>>
>> So now i'm faced with figuring out how best to solve this so that the
>> custom build step in this CMakeLists.txt that runs devenv doesn't fail:
>>
>> devenv "${CMAKE_CURRENT_SOURCE_DIR}/ClangFormat.sln" /Build Release
>>
>> There are a few options:
>>
>> 1) Forget NuGet and just commit the referenced assemblies. This is the
>> simplest solution, but is more annoying to manage if we want to upgrade 
>> the
>> versions of these assemblies later.
>>
>> 2) Commit nuget.exe to the repo and just use it. This is simple
>> enough, but I'm not sure how people feel about committing binaries, and 
>> it
>> would be frozen at whatever version we commit.
>>
>> 3) Do some form of wget to grab the latest nuget.exe from
>> "https://nuget.org/nuget.exe; in CMake and invoke it. I'm not yet sure
>> what's the simplest way to do 

[PATCH] D27163: Introduce -f[no-]strict-return flag that controls code generation for C++'s undefined behaviour return optimisation

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

What is the justification for a platform specific default change here?


Repository:
  rL LLVM

https://reviews.llvm.org/D27163



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

[PATCH] D26335: [ms] Reinstate https://reviews.llvm.org/D14748 after https://reviews.llvm.org/D20291

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: lib/Headers/x86intrin.h:49
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__))
+__tzcnt_u32(unsigned int __X) { return __X ? __builtin_ctz(__X) : 32; }
+#ifdef __x86_64__

andreadb wrote:
> hans wrote:
> > probinson wrote:
> > > hans wrote:
> > > > I'm worried about the conditional here. IIRC, ffmpeg uses TZCNT just as 
> > > > a faster encoding for BSF, but now we're putting a conditional in the 
> > > > way, so this will be slower actually. On the other hand, the 
> > > > alternative is weird too :-/
> > > I thought there was a peephole to notice a guard like this and do the 
> > > right thing? In which case having the guard is fine.
> > But for non-BMI targets it won't be able to remove the guard (unless it can 
> > prove the argument is zero).
> > 
> > MSVC will just emit the raw 'tzcnt' instruction here, and that's what 
> > ffmpeg wants.
> I understand your point. However, the semantic of tzcnt_u32 (at least for 
> BMI) is well defined on zero input. Changing that semantic for non-BMI 
> targets sounds a bit odd/confusing to me.
> 
> I guess ffmpeg is reliant on the fact that other compilers would either 
> preserve the intrinsic call until instruction selection, or fold it to a 
> meaningful value (i.e. 32 in this case) when the input is known to be zero. 
> If we remove the zero check from tzcnt_u32 (for non BMI targets), we let the 
> optimizer enable rules to aggressively fold calls to tzcnt_u32 to undef when 
> the input is zero.
> 
> As a side note: I noticed that gcc provides intrinsic `__bsfd` in 
> ia32intrin.h. Maybe we should do something similar?
Yes, ffmpeg relies on the compiler to emit the raw TZCNT instruction even for 
non-BMI targets and don't call it with zero inputs. They just want a faster BSF.

It's all pretty weird, but maybe Nico's original patch is the way to do this 
after all. At least then, the intrinsic will always do what it says in the doc.


https://reviews.llvm.org/D26335



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