[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:14115
 
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.

aaron.ballman wrote:
> Moving this function to another spot in the file makes comparing the new and 
> old code pretty hard -- would it make more sense to forward declare the 
> needed static functions so this can stay where it was and it's easier to see 
> what's changed?
@aaron.ballman I've made the required changes. Please have a look, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

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

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 486897.
fahadnayyar marked an inline comment as done.
fahadnayyar added a comment.
Herald added a project: libunwind.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libunwind.

Fixed libcxx failure and added test case for expected warning for shift assigsn 
operator.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c
  libunwind/src/AddressSpace.hpp

Index: libunwind/src/AddressSpace.hpp
===
--- libunwind/src/AddressSpace.hpp
+++ libunwind/src/AddressSpace.hpp
@@ -246,7 +246,7 @@
 inline int64_t LocalAddressSpace::getSLEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
   const uint8_t *pend = (uint8_t *)end;
-  int64_t result = 0;
+  uint64_t result = 0;
   int bit = 0;
   uint8_t byte;
   do {
@@ -260,7 +260,7 @@
   if ((byte & 0x40) != 0 && bit < 64)
 result |= (-1ULL) << bit;
   addr = (pint_t) p;
-  return result;
+  return (int64_t)result;
 }
 
 inline LocalAddressSpace::pint_t
Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,6 +13374,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13392,8 +13397,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13682,9 +13695,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, boo

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

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

In D139114#4023456 , @aaron.ballman 
wrote:

> It looks like this change breaks libc++ (see the precommit CI failures) by 
> making more changes than expected. I'm now seeing `implicit conversion 
> changes signedness` diagnostics where we didn't previously get them. Is that 
> expected and intentional? (I think it may be a fix: 
> https://godbolt.org/z/hTaaf8c5P so I'm adding the libc++ folks just in case 
> they disagree.)
>
> Also, these changes should come with an entry in the release notes.

I've made some change to suppress the warning in libcxx, please have a look and 
let me know if the change can break the semantics of the function in any way. 
Since gcc also give warning on this example, I guess we should include this 
behaviour in clang as well.




Comment at: clang/lib/Sema/SemaChecking.cpp:13400-13401
 
+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() &&

aaron.ballman wrote:
> This comment isn't quite accurate, right? It's checking for any kind of 
> implicit conversion issue (such as changing signs even if the integer widths 
> stay the same).
Changed the comment!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

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

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 486930.
fahadnayyar marked an inline comment as done.
fahadnayyar added a comment.

Added summary in clang release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c
  libunwind/src/AddressSpace.hpp

Index: libunwind/src/AddressSpace.hpp
===
--- libunwind/src/AddressSpace.hpp
+++ libunwind/src/AddressSpace.hpp
@@ -246,7 +246,7 @@
 inline int64_t LocalAddressSpace::getSLEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
   const uint8_t *pend = (uint8_t *)end;
-  int64_t result = 0;
+  uint64_t result = 0;
   int bit = 0;
   uint8_t byte;
   do {
@@ -260,7 +260,7 @@
   if ((byte & 0x40) != 0 && bit < 64)
 result |= (-1ULL) << bit;
   addr = (pint_t) p;
-  return result;
+  return (int64_t)result;
 }
 
 inline LocalAddressSpace::pint_t
Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,6 +13374,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13392,8 +13397,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13682,9 +13695,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, bool *ICContext,
+bool IsListInit) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13402
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())

arichardson wrote:
> Why is this restricted to integers? It looks like `CheckImplicitConversion` 
> also handles other types? Does calling it unconditionally cause any issues?
@arichardson since `AnalyzeCompoundAssignment` already does some custom checks 
for certain data types like floats, I added the change only for integers as I 
am trying to fix the behaviour only for integral operands in this patch. 
Otherwise we see more than one (required) warning for floats operands and also 
for shift assign operator.

This patch targets only integral changes like test 3-5 in `conversion-64-32.c`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

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

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 487955.
fahadnayyar retitled this revision from "[Clang][Sema] Enabled implicit 
conversion warning for CompoundAssignment operator. " to "[Clang][Sema] 
Enabled implicit conversion warning for CompoundAssignment operator.".
fahadnayyar added a comment.

Rebasing on main.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c
  libunwind/src/AddressSpace.hpp

Index: libunwind/src/AddressSpace.hpp
===
--- libunwind/src/AddressSpace.hpp
+++ libunwind/src/AddressSpace.hpp
@@ -246,7 +246,7 @@
 inline int64_t LocalAddressSpace::getSLEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
   const uint8_t *pend = (uint8_t *)end;
-  int64_t result = 0;
+  uint64_t result = 0;
   int bit = 0;
   uint8_t byte;
   do {
@@ -260,7 +260,7 @@
   if ((byte & 0x40) != 0 && bit < 64)
 result |= (-1ULL) << bit;
   addr = (pint_t) p;
-  return result;
+  return (int64_t)result;
 }
 
 inline LocalAddressSpace::pint_t
Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13508,6 +13508,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13526,8 +13531,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13816,9 +13829,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+Sour

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 488246.
fahadnayyar added a comment.

Moving the libunwind change to a separate patch and some refactoring.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c

Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13508,6 +13508,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13526,8 +13531,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13816,9 +13829,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, bool *ICContext,
+bool IsListInit) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -351,6 +351,9 @@
 
 Improvements to Clang's diagnostics
 ^^^
+- Clang now supports implicit conversion warnings (``-Wsign-conversion``,
+  ``Wshorten-64-to-32``, etc) for compound assignment operators (like +=, -=, <<=, >>= etc) 
+  with integral operands.
 - Clang will now check compile-time determinable string literals as format strings.
   Fixes `Issue 55805: 

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar added a comment.

Moved the libunwind change to this separate patch: 
https://reviews.llvm.org/D141515


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

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

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 488632.
fahadnayyar marked an inline comment as done.
fahadnayyar added a comment.

Refactoring.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c

Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13508,8 +13508,13 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
+/// floating-point precision and for implicit conversions for integral operands.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
   assert(isa(E) &&
  "Must be compound assignment operation");
@@ -13526,8 +13531,17 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  // FIXME: should this handle floating-point as well?
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13816,9 +13830,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, bool *ICContext,
+bool IsListInit) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -351,6 +351,9 @@
 
 Improvements to Clang's diagnostics
 ^^^
+- Clang now supports implicit conversion warnings (``-Wsign-conversion``,
+  ``Wshorten-64-to-32``, etc) for compound assignment operators (li

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c37671a7c94: [Clang][Sema] Enabled implicit conversion 
warning for CompoundAssignment… (authored by fahadnayyar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c

Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,36 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test6(char c) {
+  c <<= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+  c >>= 99; // expected-warning {{shift count >= width of type}} \
+  // cxx17-warning {{shift count >= width of type}} \
+  // ref-warning {{shift count >= width of type}} \
+  // ref-cxx17-warning {{shift count >= width of type}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13508,8 +13508,13 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
+/// floating-point precision and for implicit conversions for integral operands.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
   assert(isa(E) &&
  "Must be compound assignment operation");
@@ -13526,8 +13531,17 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversions for compound assignment statements with
+  // intergral operands.
+  // FIXME: should this handle floating-point as well?
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13816,9 +13830,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, bool *ICContext,
+bool IsListInit) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -351,6 +351,9 @@
 
 Improvements to Clang's diagnostics
 ^^^
+- Clang now suppo

[PATCH] D141868: [Clang] [Sema] Removed a fix-it for system headers

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar created this revision.
Herald added a subscriber: arphaman.
Herald added a project: All.
fahadnayyar requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Disabled an invalid fix-it which suggested fixes to be applied in system 
headers for some programs in IDEs like Xcode.

rdar://100890960


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141868

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Index/fixit-sys-header.h
  clang/test/Index/fixit-sysheader-test.cpp
  clang/test/Index/fixit-user-header.h


Index: clang/test/Index/fixit-user-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-user-header.h
@@ -0,0 +1,4 @@
+int func_in_user_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/test/Index/fixit-sysheader-test.cpp
===
--- /dev/null
+++ clang/test/Index/fixit-sysheader-test.cpp
@@ -0,0 +1,21 @@
+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+char* str;{};
+
+func_in_sys_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 
'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 0
+
+func_in_user_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 
'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 2
+
+return 0;
+}
Index: clang/test/Index/fixit-sys-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-sys-header.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10917,10 +10917,15 @@
 << ToTy << (unsigned)isObjectArgument << I + 1
 << (unsigned)(Cand->Fix.Kind);
 
-  // If we can fix the conversion, suggest the FixIts.
-  for (std::vector::iterator HI = Cand->Fix.Hints.begin(),
-   HE = Cand->Fix.Hints.end(); HI != HE; ++HI)
-FDiag << *HI;
+  // Check that location of Fn is not in system header (rdar://100890960).
+  if (!S.SourceMgr.isInSystemHeader(Fn->getLocation())) {
+// If we can fix the conversion, suggest the FixIts.
+for (std::vector::iterator HI = Cand->Fix.Hints.begin(),
+  HE = Cand->Fix.Hints.end();
+ HI != HE; ++HI)
+FDiag << *HI;
+  }
+
   S.Diag(Fn->getLocation(), FDiag);
 
   MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);


Index: clang/test/Index/fixit-user-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-user-header.h
@@ -0,0 +1,4 @@
+int func_in_user_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/test/Index/fixit-sysheader-test.cpp
===
--- /dev/null
+++ clang/test/Index/fixit-sysheader-test.cpp
@@ -0,0 +1,21 @@
+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+char* str;{};
+
+func_in_sys_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 0
+
+func_in_user_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 2
+
+return 0;
+}
Index: clang/test/Index/fixit-sys-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-sys-header.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10917,10 +10917,15 @@
 << ToTy << (unsigned)isObjectArgument << I + 1
 << (unsigned)(Cand->Fix.Kind);
 
-  // If we can fix the conversion, suggest the FixIts.
-  for (std::vector::iterator HI = Cand->Fix.Hints.begin(),
-   

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar added a comment.

@aaron.ballman  do you think that we should call ```CheckImplicitConversion``` 
only for arithemetic compound assignment operators like +=, -=, /=, *= and %= ?

For bitwiseAssign operators (|=, &=, ^=) and shiftAssign operators (<<= and 
>>=) we may have to check the semantics to understand what implicit conversions 
are allowed and what are not allowed and which of these we should include with 
```-Wconversion``` flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

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

[PATCH] D141868: [Clang] [Sema] Removed a fix-it for system headers

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 490618.
fahadnayyar added a comment.

Added release notes and changed a normal for-loop to range-based for-loop.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141868/new/

https://reviews.llvm.org/D141868

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Index/fixit-sys-header.h
  clang/test/Index/fixit-sysheader-test.cpp
  clang/test/Index/fixit-user-header.h


Index: clang/test/Index/fixit-user-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-user-header.h
@@ -0,0 +1,4 @@
+int func_in_user_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/test/Index/fixit-sysheader-test.cpp
===
--- /dev/null
+++ clang/test/Index/fixit-sysheader-test.cpp
@@ -0,0 +1,21 @@
+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+char* str;{};
+
+func_in_sys_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 
'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 0
+
+func_in_user_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 
'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 2
+
+return 0;
+}
Index: clang/test/Index/fixit-sys-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-sys-header.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10917,10 +10917,13 @@
 << ToTy << (unsigned)isObjectArgument << I + 1
 << (unsigned)(Cand->Fix.Kind);
 
-  // If we can fix the conversion, suggest the FixIts.
-  for (std::vector::iterator HI = Cand->Fix.Hints.begin(),
-   HE = Cand->Fix.Hints.end(); HI != HE; ++HI)
-FDiag << *HI;
+  // Check that location of Fn is not in system header.
+  if (!S.SourceMgr.isInSystemHeader(Fn->getLocation())) {
+// If we can fix the conversion, suggest the FixIts.
+for (const FixItHint &HI : Cand->Fix.Hints)
+FDiag << HI;
+  }
+
   S.Diag(Fn->getLocation(), FDiag);
 
   MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -353,6 +353,7 @@
 
 Improvements to Clang's diagnostics
 ^^^
+- Disabled FIT-IT suggested for a case of bad conversion in system headers.
 - Clang will now check compile-time determinable string literals as format 
strings.
   Fixes `Issue 55805: `_.
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of


Index: clang/test/Index/fixit-user-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-user-header.h
@@ -0,0 +1,4 @@
+int func_in_user_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/test/Index/fixit-sysheader-test.cpp
===
--- /dev/null
+++ clang/test/Index/fixit-sysheader-test.cpp
@@ -0,0 +1,21 @@
+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+char* str;{};
+
+func_in_sys_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 0
+
+func_in_user_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 2
+
+return 0;
+}
Index: clang/test/Index/fixit-sys-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-sys-header.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/lib/Sema/SemaOverload.cpp
===

[PATCH] D141868: [Clang] [Sema] Removed a fix-it for system headers

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar marked 3 inline comments as done.
fahadnayyar added a comment.

In D141868#4060757 , @NoQ wrote:

> Looks great! Sounds like you're looking for a more permanent fix, I guess 
> `ConversionFixItGenerator` could try to avoid adding fixits to system header 
> functions?

It probably requires some larger rearchitecturing of the FixItGeneration 
implementation to avoid system headers for all kinds of FixitHints like 
`ConversionFixIt`. We can do that later as a separate patch after getting some 
more examples where fixits are suggested for system headers.

In D141868#4065287 , @aaron.ballman 
wrote:

> You should also add a release note for the changes.

Done!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141868/new/

https://reviews.llvm.org/D141868

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

[PATCH] D141868: [Clang] [Sema] Removed a fix-it for system headers

[Fahad Nayyar via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e5d901feb3c: [Clang] [Sema] Removed a fix-it for system 
headers (authored by fahadnayyar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141868/new/

https://reviews.llvm.org/D141868

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Index/fixit-sys-header.h
  clang/test/Index/fixit-sysheader-test.cpp
  clang/test/Index/fixit-user-header.h


Index: clang/test/Index/fixit-user-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-user-header.h
@@ -0,0 +1,4 @@
+int func_in_user_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/test/Index/fixit-sysheader-test.cpp
===
--- /dev/null
+++ clang/test/Index/fixit-sysheader-test.cpp
@@ -0,0 +1,21 @@
+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+char* str;{};
+
+func_in_sys_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 
'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 0
+
+func_in_user_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 
'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 2
+
+return 0;
+}
Index: clang/test/Index/fixit-sys-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-sys-header.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10917,10 +10917,13 @@
 << ToTy << (unsigned)isObjectArgument << I + 1
 << (unsigned)(Cand->Fix.Kind);
 
-  // If we can fix the conversion, suggest the FixIts.
-  for (std::vector::iterator HI = Cand->Fix.Hints.begin(),
-   HE = Cand->Fix.Hints.end(); HI != HE; ++HI)
-FDiag << *HI;
+  // Check that location of Fn is not in system header.
+  if (!S.SourceMgr.isInSystemHeader(Fn->getLocation())) {
+// If we can fix the conversion, suggest the FixIts.
+for (const FixItHint &HI : Cand->Fix.Hints)
+FDiag << HI;
+  }
+
   S.Diag(Fn->getLocation(), FDiag);
 
   MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -359,6 +359,7 @@
 
 Improvements to Clang's diagnostics
 ^^^
+- Disabled FIT-IT suggested for a case of bad conversion in system headers.
 - Clang will now check compile-time determinable string literals as format 
strings.
   Fixes `Issue 55805: `_.
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of


Index: clang/test/Index/fixit-user-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-user-header.h
@@ -0,0 +1,4 @@
+int func_in_user_header(char * s, unsigned long len)
+{
+return 0;
+}
Index: clang/test/Index/fixit-sysheader-test.cpp
===
--- /dev/null
+++ clang/test/Index/fixit-sysheader-test.cpp
@@ -0,0 +1,21 @@
+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+char* str;{};
+
+func_in_sys_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 0
+
+func_in_user_header(str, str + 10);
+// CHECK: Number FIX-ITs = 0
+// CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+// CHECK-NEXT: Number FIX-ITs = 2
+
+return 0;
+}
Index: clang/test/Index/fixit-sys-header.h
===
--- /dev/null
+++ clang/test/Index/fixit-sys-header.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+return 0;

[PATCH] D138434: [Clang][Sema] Added space after ',' in a warning

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar created this revision.
Herald added a project: All.
fahadnayyar requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change fixes a typo in a warning message.

rdar://79707705


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138434

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaObjC/atomic-property-synthesis-rules.m


Index: clang/test/SemaObjC/atomic-property-synthesis-rules.m
===
--- clang/test/SemaObjC/atomic-property-synthesis-rules.m
+++ clang/test/SemaObjC/atomic-property-synthesis-rules.m
@@ -108,7 +108,7 @@
 // read-write - might warn
 @property int GetSet;
 @property int Get; // expected-note {{property declared here}} \
-// expected-note {{setter and getter must both be 
synthesized}}
+// expected-note {{setter and getter must both be 
synthesized, or both be user defined, or the property must be nonatomic}}
 @property int Set; // expected-note {{property declared here}} \
 // expected-note {{setter and getter must both be 
synthesized}}
 @property int None;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1326,7 +1326,7 @@
   "with a user defined %select{getter|setter}2">,
   InGroup>;
 def note_atomic_property_fixup_suggest : Note<"setter and getter must both be "
-  "synthesized, or both be user defined,or the property must be nonatomic">;
+  "synthesized, or both be user defined, or the property must be nonatomic">;
 def err_atomic_property_nontrivial_assign_op : Error<
   "atomic property of reference type %0 cannot have non-trivial assignment"
   " operator">;


Index: clang/test/SemaObjC/atomic-property-synthesis-rules.m
===
--- clang/test/SemaObjC/atomic-property-synthesis-rules.m
+++ clang/test/SemaObjC/atomic-property-synthesis-rules.m
@@ -108,7 +108,7 @@
 // read-write - might warn
 @property int GetSet;
 @property int Get;	// expected-note {{property declared here}} \
-// expected-note {{setter and getter must both be synthesized}}
+// expected-note {{setter and getter must both be synthesized, or both be user defined, or the property must be nonatomic}}
 @property int Set;	// expected-note {{property declared here}} \
 // expected-note {{setter and getter must both be synthesized}}
 @property int None;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1326,7 +1326,7 @@
   "with a user defined %select{getter|setter}2">,
   InGroup>;
 def note_atomic_property_fixup_suggest : Note<"setter and getter must both be "
-  "synthesized, or both be user defined,or the property must be nonatomic">;
+  "synthesized, or both be user defined, or the property must be nonatomic">;
 def err_atomic_property_nontrivial_assign_op : Error<
   "atomic property of reference type %0 cannot have non-trivial assignment"
   " operator">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator. This change enables Wshorten-64-to-32 waring for compound assignment statements which has implicit conversion

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar created this revision.
Herald added a project: All.
fahadnayyar requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...://10466193


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139114

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c

Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,25 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,44 +13374,6 @@
   }
 }
 
-/// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
-  assert(isa(E) &&
- "Must be compound assignment operation");
-  // Recurse on the LHS and RHS in here
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
-  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
-
-  if (E->getLHS()->getType()->isAtomicType())
-S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
-
-  // If source is floating point but target is an integer.
-  if (ResultBT->isInteger())
-return DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
-   E->getExprLoc(), diag::warn_impcast_float_integer);
-
-  if (!ResultBT->isFloatingPoint())
-return;
-
-  // If both source and target are floating points, warn about losing precision.
-  int Order = S.getASTContext().getFloatingTypeSemanticOrder(
-  QualType(ResultBT, 0), QualType(RBT, 0));
-  if (Order < 0 && !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
-// warn about dropping FP rank.
-DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(),
-diag::warn_impcast_float_result_precision);
-}
-
 static std::string PrettyPrintInRange(const llvm::APSInt &Value,
   IntRange Range) {
   if (!Range.Width) return "0";
@@ -14150,6 +14112,49 @@
   }
 }
 
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.
+static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
+  assert(isa(E) &&
+ "Must be compound assignment operation");
+  // Recurse on the LHS and RHS in here
+  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+
+  if (E->getLHS()->getType()->isAtomicType())
+S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
+
+  // Now check the outermost expression
+  const auto *ResultBT = E->getLHS()->getType()->getAs();
+  const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+
+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() && E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(), E->getRHS()->getExprLoc());
+
+  // The below checks assume source is floating point.
+  if

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 479626.
fahadnayyar retitled this revision from "[Clang][Sema] Enabled 
Wshorten-64-to-32 for CompoundAssignment operator. " to "[Clang][Sema] Enabled 
Wshorten-64-to-32 for CompoundAssignment operator.".
fahadnayyar added a comment.

Fixing clang-format error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c

Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,25 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer precision}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,44 +13374,6 @@
   }
 }
 
-/// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
-  assert(isa(E) &&
- "Must be compound assignment operation");
-  // Recurse on the LHS and RHS in here
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
-  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
-
-  if (E->getLHS()->getType()->isAtomicType())
-S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
-
-  // If source is floating point but target is an integer.
-  if (ResultBT->isInteger())
-return DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
-   E->getExprLoc(), diag::warn_impcast_float_integer);
-
-  if (!ResultBT->isFloatingPoint())
-return;
-
-  // If both source and target are floating points, warn about losing precision.
-  int Order = S.getASTContext().getFloatingTypeSemanticOrder(
-  QualType(ResultBT, 0), QualType(RBT, 0));
-  if (Order < 0 && !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
-// warn about dropping FP rank.
-DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(),
-diag::warn_impcast_float_result_precision);
-}
-
 static std::string PrettyPrintInRange(const llvm::APSInt &Value,
   IntRange Range) {
   if (!Range.Width) return "0";
@@ -14150,6 +14112,51 @@
   }
 }
 
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.
+static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
+  assert(isa(E) &&
+ "Must be compound assignment operation");
+  // Recurse on the LHS and RHS in here
+  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+
+  if (E->getLHS()->getType()->isAtomicType())
+S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
+
+  // Now check the outermost expression
+  const auto *ResultBT = E->getLHS()->getType()->getAs();
+  const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+
+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerTyp

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar created this revision.
Herald added a project: All.
fahadnayyar requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

When libc++ is bootstrapped with clang using the cmake options
-DLLVM_ENABLE_PROJECTS="clang;llvm;lldb" and 
-DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" then the resulting clang uses 
the just-built libcxx headers from /bin/../include/c++/v1 
. So the clang in this case should also use the just-built libc++.dylib 
from /bin/../lib instead of using the system's libc++ 
libraries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148266

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -410,6 +410,21 @@
   Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
   Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
 
+  // Including the path to the just-built libc++.dylib if libc++ is 
bootstrapped
+  // and /bin/../lib/libc++.dylib is a valid path
+
+  llvm::SmallString<128> LibCXXDylibDirPath =
+  llvm::StringRef(D.getInstalledDir()); // /bin
+  llvm::sys::path::append(LibCXXDylibDirPath, "..", "lib");
+
+  llvm::SmallString<128> LibCXXDylibPath = LibCXXDylibDirPath;
+  llvm::sys::path::append(LibCXXDylibPath, "..", "lib", "libc++.dylib");
+
+  if (D.getVFS().exists(LibCXXDylibPath)) {
+CmdArgs.push_back("-L");
+CmdArgs.push_back(C.getArgs().MakeArgString(LibCXXDylibDirPath));
+  }
+
   // Give --sysroot= preference, over the Apple specific behavior to also use
   // --isysroot as the syslibroot.
   StringRef sysroot = C.getSysRoot();


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -410,6 +410,21 @@
   Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
   Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
 
+  // Including the path to the just-built libc++.dylib if libc++ is bootstrapped
+  // and /bin/../lib/libc++.dylib is a valid path
+
+  llvm::SmallString<128> LibCXXDylibDirPath =
+  llvm::StringRef(D.getInstalledDir()); // /bin
+  llvm::sys::path::append(LibCXXDylibDirPath, "..", "lib");
+
+  llvm::SmallString<128> LibCXXDylibPath = LibCXXDylibDirPath;
+  llvm::sys::path::append(LibCXXDylibPath, "..", "lib", "libc++.dylib");
+
+  if (D.getVFS().exists(LibCXXDylibPath)) {
+CmdArgs.push_back("-L");
+CmdArgs.push_back(C.getArgs().MakeArgString(LibCXXDylibDirPath));
+  }
+
   // Give --sysroot= preference, over the Apple specific behavior to also use
   // --isysroot as the syslibroot.
   StringRef sysroot = C.getSysRoot();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 514037.
fahadnayyar added a comment.
Herald added a subscriber: ormris.

Added test case. Now checking the existence of libc++ headers in the toolchain 
when including the libc++.dylib form toolchain and vice versa. Also checking 
the absence of -nostdinc, -nostdinc++ or -nostdlib 
arguments before linking with libc++.dylib from the toolchain.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp

Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -1,63 +1,49 @@
 // UNSUPPORTED: system-windows
 
-// General tests that the header search paths for libc++ detected by the driver
-// and passed to CC1 are correct on Darwin platforms.
+// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib 
+// whenever we are including the toolchain's libc++ headers
 
 // Check without a sysroot and without headers alongside the installation
-// (no include path should be added, and no warning or error).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s
-// CHECK-LIBCXX-NONE: "-cc1"
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with only headers alongside the installation (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: --sysroot="" \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
-// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
-// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with only headers in the sysroot (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
-// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the  headers).
-// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence
-// over --sysroot.
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
@@ -65,9 +51,9 @@
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_us

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
   if (sysroot != "") {
 CmdArgs.push_back("-syslibroot");
 CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));

ldionne wrote:
> ldionne wrote:
> > Where do we set the usual `/usr/lib` search path? I think it might 
> > make sense to move the code you added to that place.
> What I mean: there's a place where we must be adding `-L /usr/lib` 
> in the code, and it isn't in `darwin::Linker::AddLinkArgs`. Where is it? 
> Would it make sense to move the code you added to that place instead?
/usr/local/include is added here in 
```DarwinClang::AddClangSystemIncludeArgs```: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2346
 and /usr/include is added here: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2374
 .. But since we want to add a new argument to the linker invocation, I feel 
```darwin::Linker::AddLinkArgs``` is the best place. But let me know if you 
think otherwise.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

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

[PATCH] D150988: [clang][Darwin] Error out when missing requested libarclite library

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 525791.
fahadnayyar added a comment.

Adding 1 more test chage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150988/new/

https://reviews.llvm.org/D150988

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/arclite-link-external-toolchain.c
  clang/test/Driver/arclite-link.c


Index: clang/test/Driver/arclite-link.c
===
--- clang/test/Driver/arclite-link.c
+++ clang/test/Driver/arclite-link.c
@@ -1,9 +1,13 @@
 // RUN: touch %t.o
-// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo 
-mmacosx-version-min=10.10 %t.o 2>&1 | FileCheck 
-check-prefix=CHECK-ARCLITE-OSX %s
+// RUN: mkdir -p 
%t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
+// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo 
-mmacosx-version-min=10.10 %t.o \
+// RUN: -isysroot 
%t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
 2>&1 | FileCheck -check-prefix=CHECK-ARCLITE-OSX %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime 
-mmacosx-version-min=10.11 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE 
%s
 // RUN: %clang -### -target i386-apple-darwin10 -fobjc-link-runtime 
-mmacosx-version-min=10.7 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime 
-nostdlib %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOSTDLIB %s
 
+// CHECK-ARCLITE-OSX: .tmpdir/Xcode.app/{{.*}}libarclite_macosx.a'.
+// CHECK-ARCLITE-OSX: This likely means you need to increase your minimum 
deployment target
 // CHECK-ARCLITE-OSX: -lfoo
 // CHECK-ARCLITE-OSX: libarclite_macosx.a
 // CHECK-ARCLITE-OSX: -framework
Index: clang/test/Driver/arclite-link-external-toolchain.c
===
--- clang/test/Driver/arclite-link-external-toolchain.c
+++ clang/test/Driver/arclite-link-external-toolchain.c
@@ -4,5 +4,8 @@
 // RUN:   -isysroot 
%t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
 \
 // RUN:   %s 2>&1 | FileCheck %s
 
+// CHECK: error: SDK does not contain 'libarclite' at the path '
+// CHECK: .tmpdir/Xcode.app/{{.*}}libarclite_macosx.a'.
+// CHECK: This likely means you need to increase your minimum deployment target
 // CHECK: -lfoo
 // CHECK: .tmpdir/Xcode.app/{{.*}}libarclite_macosx.a
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1204,6 +1204,9 @@
 P += "macosx";
   P += ".a";
 
+  if (!getVFS().exists(P))
+getDriver().Diag(clang::diag::err_drv_darwin_sdk_missing_arclite) << P;
+
   CmdArgs.push_back(Args.MakeArgString(P));
 }
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -619,6 +619,9 @@
   "SDK settings were ignored as 'SDKSettings.json' could not be parsed">,
   InGroup>;
 
+def err_drv_darwin_sdk_missing_arclite : Error<
+  "SDK does not contain 'libarclite' at the path '%0'. This likely means you 
need to increase your minimum deployment target">;
+
 def err_drv_trivial_auto_var_init_stop_after_missing_dependency : Error<
   "'-ftrivial-auto-var-init-stop-after=*' is used without "
   "'-ftrivial-auto-var-init=zero' or '-ftrivial-auto-var-init=pattern'">;


Index: clang/test/Driver/arclite-link.c
===
--- clang/test/Driver/arclite-link.c
+++ clang/test/Driver/arclite-link.c
@@ -1,9 +1,13 @@
 // RUN: touch %t.o
-// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo -mmacosx-version-min=10.10 %t.o 2>&1 | FileCheck -check-prefix=CHECK-ARCLITE-OSX %s
+// RUN: mkdir -p %t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
+// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo -mmacosx-version-min=10.10 %t.o \
+// RUN: -isysroot %t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk 2>&1 | FileCheck -check-prefix=CHECK-ARCLITE-OSX %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -mmacosx-version-min=10.11 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE %s
 // RUN: %clang -### -target i386-apple-darwin10 -fobjc-link-runtime -mmacosx-version-min=10.7 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -nostdlib %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOSTDLIB %s
 
+// CHECK

[PATCH] D150988: [clang][Darwin] Error out when missing requested libarclite library

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 526596.
fahadnayyar added a comment.

Rebasing to main.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150988/new/

https://reviews.llvm.org/D150988

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/arclite-link-external-toolchain.c
  clang/test/Driver/arclite-link.c


Index: clang/test/Driver/arclite-link.c
===
--- clang/test/Driver/arclite-link.c
+++ clang/test/Driver/arclite-link.c
@@ -1,9 +1,13 @@
 // RUN: touch %t.o
-// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo 
-mmacosx-version-min=10.10 %t.o 2>&1 | FileCheck 
-check-prefix=CHECK-ARCLITE-OSX %s
+// RUN: mkdir -p 
%t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
+// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo 
-mmacosx-version-min=10.10 %t.o \
+// RUN: -isysroot 
%t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
 2>&1 | FileCheck -check-prefix=CHECK-ARCLITE-OSX %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime 
-mmacosx-version-min=10.11 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE 
%s
 // RUN: %clang -### -target i386-apple-darwin10 -fobjc-link-runtime 
-mmacosx-version-min=10.7 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime 
-nostdlib %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOSTDLIB %s
 
+// CHECK-ARCLITE-OSX: .tmpdir/Xcode.app/{{.*}}libarclite_macosx.a'.
+// CHECK-ARCLITE-OSX: This likely means you need to increase your minimum 
deployment target
 // CHECK-ARCLITE-OSX: -lfoo
 // CHECK-ARCLITE-OSX: libarclite_macosx.a
 // CHECK-ARCLITE-OSX: -framework
Index: clang/test/Driver/arclite-link-external-toolchain.c
===
--- clang/test/Driver/arclite-link-external-toolchain.c
+++ clang/test/Driver/arclite-link-external-toolchain.c
@@ -4,5 +4,8 @@
 // RUN:   -isysroot 
%t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
 \
 // RUN:   %s 2>&1 | FileCheck %s
 
+// CHECK: error: SDK does not contain 'libarclite' at the path '
+// CHECK: .tmpdir/Xcode.app/{{.*}}libarclite_macosx.a'.
+// CHECK: This likely means you need to increase your minimum deployment target
 // CHECK: -lfoo
 // CHECK: .tmpdir/Xcode.app/{{.*}}libarclite_macosx.a
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1204,6 +1204,9 @@
 P += "macosx";
   P += ".a";
 
+  if (!getVFS().exists(P))
+getDriver().Diag(clang::diag::err_drv_darwin_sdk_missing_arclite) << P;
+
   CmdArgs.push_back(Args.MakeArgString(P));
 }
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -619,6 +619,9 @@
   "SDK settings were ignored as 'SDKSettings.json' could not be parsed">,
   InGroup>;
 
+def err_drv_darwin_sdk_missing_arclite : Error<
+  "SDK does not contain 'libarclite' at the path '%0'. This likely means you 
need to increase your minimum deployment target">;
+
 def err_drv_trivial_auto_var_init_stop_after_missing_dependency : Error<
   "'-ftrivial-auto-var-init-stop-after=*' is used without "
   "'-ftrivial-auto-var-init=zero' or '-ftrivial-auto-var-init=pattern'">;


Index: clang/test/Driver/arclite-link.c
===
--- clang/test/Driver/arclite-link.c
+++ clang/test/Driver/arclite-link.c
@@ -1,9 +1,13 @@
 // RUN: touch %t.o
-// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo -mmacosx-version-min=10.10 %t.o 2>&1 | FileCheck -check-prefix=CHECK-ARCLITE-OSX %s
+// RUN: mkdir -p %t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
+// RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -lfoo -mmacosx-version-min=10.10 %t.o \
+// RUN: -isysroot %t.tmpdir/Xcode.app/Contents/Developers/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk 2>&1 | FileCheck -check-prefix=CHECK-ARCLITE-OSX %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -mmacosx-version-min=10.11 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE %s
 // RUN: %clang -### -target i386-apple-darwin10 -fobjc-link-runtime -mmacosx-version-min=10.7 %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOARCLITE %s
 // RUN: %clang -### -target x86_64-apple-darwin10 -fobjc-link-runtime -nostdlib %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOSTDLIB %s
 
+// CHECK-ARCLITE

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 527241.
fahadnayyar added a comment.

Passing -L /bin/../lib/ unconditionally to the linker.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp

Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -1,63 +1,48 @@
 // UNSUPPORTED: system-windows
 
-// General tests that the header search paths for libc++ detected by the driver
-// and passed to CC1 are correct on Darwin platforms.
+// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
 
 // Check without a sysroot and without headers alongside the installation
-// (no include path should be added, and no warning or error).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s
-// CHECK-LIBCXX-NONE: "-cc1"
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with only headers alongside the installation (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: --sysroot="" \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
-// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
-// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with only headers in the sysroot (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
-// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the  headers).
-// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence
-// over --sysroot.
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
@@ -65,9 +50,9 @@
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-ap

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 527407.
fahadnayyar added a comment.

Rebasing to latest main.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp

Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -1,63 +1,48 @@
 // UNSUPPORTED: system-windows
 
-// General tests that the header search paths for libc++ detected by the driver
-// and passed to CC1 are correct on Darwin platforms.
+// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
 
 // Check without a sysroot and without headers alongside the installation
-// (no include path should be added, and no warning or error).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s
-// CHECK-LIBCXX-NONE: "-cc1"
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with only headers alongside the installation (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: --sysroot="" \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
-// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
-// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with only headers in the sysroot (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
-// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the  headers).
-// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence
-// over --sysroot.
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
@@ -65,9 +50,9 @@
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -std

[PATCH] D150988: [clang][Darwin] Error out when missing requested libarclite library

[Fahad Nayyar via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150988/new/

https://reviews.llvm.org/D150988

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

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 527512.
fahadnayyar added a comment.

Simplifying the test case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp


Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to 
prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the 
toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir 
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// Check with -stdlib on both cases of toolchain installations (with and 
without libcxx).
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir 
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx 
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain 
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "/usr/bin/ld"
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "-L" "[[TOOLCHAIN]]/usr/bin/../lib"
Index: clang/test/Driver/darwin-header-search-libcxx.cpp
===
--- clang/test/Driver/darwin-header-search-libcxx.cpp
+++ clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -116,7 +116,7 @@
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN: -stdlib=platform \
 // RUN: -nostdinc++ \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
 // CHECK-LIBCXX-NOSTDINCXX: "-cc1"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -417,6 +417,17 @@
   Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
   Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
 
+  // Pass -L /bin/../lib/ to linker to prioritize 
toolchain's
+  // libc++.dylib over the sysroot-provided one. This matches what we do for
+  // determining which libc++ headers to use.
+  llvm::SmallString<128> InstallBinPath =
+  llvm::StringRef(D.getInstalledDir()); // /bin
+  llvm::SmallString<128> LibCxxDylibDirPath = InstallBinPath;
+  llvm::sys::path::append(LibCxxDylibDirPath, "..",
+  "lib"); // /bin/../lib
+  CmdArgs.push_back("-L");
+  CmdArgs.push_back(C.getArgs().MakeArgString(LibCxxDylibDirPath));
+
   // Give --sysroot= preference, over the Apple specific behavior to also use
   // --isysroot as the syslibroot.
   StringRef sysroot = C.getSysRoot();


Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// Check with -stdlib on both cases of toolchain installations (with and without libcxx).
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_lib

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/darwin-header-search-libcxx.cpp:119
 // RUN: -nostdinc++ \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \

ldionne wrote:
> Is this change really needed anymore? Why?
This is unrelated small bug in darwin-header-search-libcxx.cpp. I thought maybe 
we fix that small issue also in this patch. What you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

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

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 527591.
fahadnayyar added a comment.

Edits in comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp


Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to 
prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the 
toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir 
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// Check with -stdlib on both cases of toolchain installations (with and 
without libcxx).
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir 
%S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx 
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -stdlib=libc++ \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain 
--check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
+
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "/usr/bin/ld"
+// CHECK-TOOLCHAIN-LIBCXX-LINKING-1: "-L" "[[TOOLCHAIN]]/usr/bin/../lib"
Index: clang/test/Driver/darwin-header-search-libcxx.cpp
===
--- clang/test/Driver/darwin-header-search-libcxx.cpp
+++ clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -92,7 +92,7 @@
 // CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" 
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
 // CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/v1"
 
-// Make sure that using -nostdinc, -nostdinc++ or -nostdlib will drop both the 
toolchain
+// Make sure that using -nostdinc, -nostdinc++ or -nostdlibinc will drop both 
the toolchain
 // C++ include path and the sysroot one.
 //
 // RUN: %clang -### %s -fsyntax-only 2>&1 \
@@ -116,7 +116,7 @@
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN: -stdlib=platform \
 // RUN: -nostdinc++ \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr \
+// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   --check-prefix=CHECK-LIBCXX-NOSTDINCXX %s
 // CHECK-LIBCXX-NOSTDINCXX: "-cc1"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -417,6 +417,17 @@
   Args.AddAllArgs(CmdArgs, options::OPT_sub__library);
   Args.AddAllArgs(CmdArgs, options::OPT_sub__umbrella);
 
+  // Pass -L /bin/../lib/ to linker to prioritize 
toolchain's
+  // libc++.dylib over the sysroot-provided one. This matches what we do for
+  // determining which libc++ headers to use.
+  llvm::SmallString<128> InstallBinPath =
+  llvm::StringRef(D.getInstalledDir()); // /bin
+  llvm::SmallString<128> LibCxxDylibDirPath = InstallBinPath;
+  llvm::sys::path::append(LibCxxDylibDirPath, "..",
+  "lib"); // /bin/../lib
+  CmdArgs.push_back("-L");
+  CmdArgs.push_back(C.getArgs().MakeArgString(LibCxxDylibDirPath));
+
   // Give --sysroot= preference, over the Apple specific behavior to also use
   // --isysroot as the syslibroot.
   StringRef sysroot = C.getSysRoot();


Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- /dev/null
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// Tests to check that we pass -L /bin/../lib/ to the linker to prioritize the toolchain's libc++.dylib over system's libc++.tbd in all cases.
+
+// Check that even if stdlib is not passed, on apple platforms we pass the toolchain's dylib path to the linker.
+
+// RUN: %clang -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin \
+// RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
+// RUN:   | FileC

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 514397.
fahadnayyar added a comment.
Herald added a subscriber: carlosgalvezp.
Herald added a project: clang-tools-extra.

Fixed regression error in clang-tidy test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/mock-libcxx/lib/libc++.dylib
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp

Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -1,63 +1,49 @@
 // UNSUPPORTED: system-windows
 
-// General tests that the header search paths for libc++ detected by the driver
-// and passed to CC1 are correct on Darwin platforms.
+// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib 
+// whenever we are including the toolchain's libc++ headers
 
 // Check without a sysroot and without headers alongside the installation
-// (no include path should be added, and no warning or error).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s
-// CHECK-LIBCXX-NONE: "-cc1"
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with only headers alongside the installation (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: --sysroot="" \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
-// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
-// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with only headers in the sysroot (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
-// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the  headers).
-// Ensure that both -isysroot and --sysroot work, and that isysroot has precedence
-// over --sysroot.
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
@@ -65,9 +51,9 @@
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   

[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 514699.
fahadnayyar added a comment.

Fixing some more regression test errors.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148266/new/

https://reviews.llvm.org/D148266

Files:
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/mock-libcxx/lib/libc++.dylib
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/Inputs/basic_darwin_toolchain/usr/lib/libc++.dylib
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
  clang/test/Driver/stdlibxx-isystem.cpp
  clang/test/Tooling/Inputs/mock-libcxx/lib/libc++.dylib

Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- clang/test/Driver/stdlibxx-isystem.cpp
+++ clang/test/Driver/stdlibxx-isystem.cpp
@@ -5,6 +5,8 @@
 
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/lib
+// RUN: touch %t/lib/libc++.dylib
 // RUN: mkdir -p %t/include/c++/v1
 // RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
Index: clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
===
--- clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
+++ clang/test/Driver/darwin-link-libcxx-from-toolchain.cpp
@@ -1,63 +1,49 @@
 // UNSUPPORTED: system-windows
 
-// General tests that the header search paths for libc++ detected by the driver
-// and passed to CC1 are correct on Darwin platforms.
+// Tests to check that we pass -L /bin/../lib/libc++.dylib to link with the toolchain's libc++.dylib 
+// whenever we are including the toolchain's libc++ headers
 
 // Check without a sysroot and without headers alongside the installation
-// (no include path should be added, and no warning or error).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
-// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-NONE %s
-// CHECK-LIBCXX-NONE: "-cc1"
+// RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with only headers alongside the installation (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: --sysroot="" \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-1 %s
-// CHECK-LIBCXX-TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-1: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-1-NOT: "-internal-isystem" "/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_no_libcxx \
 // RUN:   | FileCheck -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain \
 // RUN:   -DSYSROOT=%S/Inputs/basic_darwin_sdk_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-TOOLCHAIN-2 %s
-// CHECK-LIBCXX-TOOLCHAIN-2: "-cc1"
-// CHECK-LIBCXX-TOOLCHAIN-2: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
-// CHECK-LIBCXX-TOOLCHAIN-2-NOT: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-1 %s
 
 // Check with only headers in the sysroot (those should be used).
 //
-// RUN: %clang -### %s -fsyntax-only 2>&1 \
+// RUN: %clang -### %s 2>&1 \
 // RUN: --target=x86_64-apple-darwin \
 // RUN: -stdlib=libc++ \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
 // RUN:   -DTOOLCHAIN=%S/Inputs/basic_darwin_toolchain_no_libcxx \
-// RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT-1 %s
-// CHECK-LIBCXX-SYSROOT-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT-1: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" "[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// RUN:   --check-prefix=CHECK-TOOLCHAIN-LIBCXX-LINKING-2 %s
 
 // Check with both headers in the sysroot and headers alongside the installation
-// (the headers in the toolchain should be preferred over the  headers).
-// Ensure that both -isysr

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

fahadnayyar updated this revision to Diff 482102.
fahadnayyar added a comment.

Added forward declare for CheckImplicitConversion to make the diff look clean.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139114/new/

https://reviews.llvm.org/D139114

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-64-32.c


Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,25 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer 
precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer 
precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer 
precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer 
precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer 
precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer 
precision}}
+  i += (i+j)*ll; // expected-warning {{implicit conversion loses integer 
precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer 
precision}}
+  i -= ll/(i-j); // expected-warning {{implicit conversion loses integer 
precision}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13374,6 +13374,11 @@
   }
 }
 
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+SourceLocation CC,
+bool *ICContext = nullptr,
+bool IsListInit = false);
+
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
 static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -13392,8 +13397,16 @@
 ->getComputationResultType()
 ->getAs();
 
+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() &&
+  E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp())
+CheckImplicitConversion(S, E->getRHS(), E->getType(),
+E->getRHS()->getExprLoc());
+
   // The below checks assume source is floating point.
-  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint())
+return;
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
@@ -13682,9 +13695,8 @@
 }
 
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
-SourceLocation CC,
-bool *ICContext = nullptr,
-bool IsListInit = false) {
+SourceLocation CC, bool *ICContext,
+bool IsListInit) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();


Index: clang/test/Sema/conversion-64-32.c
===
--- clang/test/Sema/conversion-64-32.c
+++ clang/test/Sema/conversion-64-32.c
@@ -17,3 +17,25 @@
 int test2(long v) {
   return v / 2; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}}
 }
+
+// rdar://10466193
+void test3(int i, long long ll) {
+  i += ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i *= ll; // expected-warning {{implicit conversion loses integer precision}}
+  i /= ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test4(int i, long long ll) {
+  i += i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i += i+ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i-ll; // expected-warning {{implicit conversion loses integer precision}}
+  i -= i+ll; // expected-warning {{implicit conversion loses integer precision}}
+}
+
+void test5(int i, int j, long long ll) {
+  i += (i-j)*ll; // expected-warning {{implicit conversion loses integer precision}}
+  i