[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
Herald added a subscriber: cfe-commits.

Fixes Bug: https://bugs.llvm.org/show_bug.cgi?id=27061


Repository:
  rC Clang

https://reviews.llvm.org/D50467

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-float-conversion.cpp

Index: test/SemaCXX/warn-float-conversion.cpp
===
--- test/SemaCXX/warn-float-conversion.cpp
+++ test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,22 @@
   l = ld;  //expected-warning{{conversion}}
 }
 
+void CompoundAssignment() {
+  int x = 3;
+
+  x += 1.234;  //expected-warning{{conversion}}
+  x -= -0.0;  //expected-warning{{conversion}}
+  x *= 1.1f;  //expected-warning{{conversion}}
+  x /= -2.2f;  //expected-warning{{conversion}}
+
+  int y = x += 1.4f;  //expected-warning{{conversion}}
+
+  float z = 1.1f;
+  double w = -2.2;
+
+  y += z + w;  //expected-warning{{conversion}}
+}
+
 void Test() {
   int a1 = 10.0/2.0;  //expected-warning{{conversion}}
   int a2 = 1.0/2.0;  //expected-warning{{conversion}}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10292,33 +10292,6 @@
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
-/// 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());
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // If both source and target are floating points.
-  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
-  // We don't want to warn for system macro.
-  if (!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);
-}
-
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
 SourceLocation CContext) {
@@ -10421,6 +10394,38 @@
   }
 }
 
+/// 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());
+
+  // Now check the outermost expression
+  const auto *ResultBT = E->getLHS()->getType()->getAs();
+  const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+  if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return;
+
+  // If both source and target are floating points.
+  if (ResultBT->isFloatingPoint()) {
+// Builtin FP kinds are ordered by increasing FP rank.
+if(ResultBT->getKind() < RBT->getKind() &&
+   // We don't want to warn for system macro.
+   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);
+
+  } else
+// If source is floating point but target is not.
+DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), E->getExprLoc());
+}
+
 static std::string PrettyPrintInRange(const llvm::APSInt &Value,
   IntRange Range) {
   if (!Range.Width) return "0";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 159763.
nickdesaulniers added a comment.

- rework ordering of conditionals to reduce indentation


Repository:
  rC Clang

https://reviews.llvm.org/D50467

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-float-conversion.cpp

Index: test/SemaCXX/warn-float-conversion.cpp
===
--- test/SemaCXX/warn-float-conversion.cpp
+++ test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,22 @@
   l = ld;  //expected-warning{{conversion}}
 }
 
+void CompoundAssignment() {
+  int x = 3;
+
+  x += 1.234;  //expected-warning{{conversion}}
+  x -= -0.0;  //expected-warning{{conversion}}
+  x *= 1.1f;  //expected-warning{{conversion}}
+  x /= -2.2f;  //expected-warning{{conversion}}
+
+  int y = x += 1.4f;  //expected-warning{{conversion}}
+
+  float z = 1.1f;
+  double w = -2.2;
+
+  y += z + w;  //expected-warning{{conversion}}
+}
+
 void Test() {
   int a1 = 10.0/2.0;  //expected-warning{{conversion}}
   int a2 = 1.0/2.0;  //expected-warning{{conversion}}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10292,33 +10292,6 @@
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
-/// 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());
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // If both source and target are floating points.
-  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
-  // We don't want to warn for system macro.
-  if (!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);
-}
-
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
 SourceLocation CContext) {
@@ -10421,6 +10394,37 @@
   }
 }
 
+/// 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());
+
+  // Now check the outermost expression
+  const auto *ResultBT = E->getLHS()->getType()->getAs();
+  const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+  if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return;
+
+  // If source is floating point but target is not.
+  if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+   E->getExprLoc());
+
+  // If both source and target are floating points.
+  // Builtin FP kinds are ordered by increasing FP rank.
+  if (ResultBT->getKind() < RBT->getKind() &&
+  // We don't want to warn for system macro.
+  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";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10413-10416
+  // If source is floating point but target is not.
+  if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+   E->getExprLoc());

This is essentially the only new code.  AnalyzeCompoundAssignment was moved in 
order to be able to call DiagnoseFloatingImpCast (it's defined above). The rest 
is a small refactoring of the conditionals.


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10411
+->getAs();
+  if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return;
+

pirama wrote:
> Add a comment explaining this conditional as well?  
> 
> > Return if source and target types are unavailable or if source is not a 
> > floating point.
> 
> With the comment, it might be cleaner to read if you expand the negation: 
> `!ResultBT || !RBT || !RBT->isFloatingPoint()`
`ResultBT` and `RBT` are pointers that may be null and need to be checked.  The 
previous code did this before accessing them, I've just moved the checks to be 
sooner.

If `!RBT` (if it's `nullptr`), then `!RBT->isFloatingPoint()` would be a null 
deref, so unfortunately I can not expand it as recommended.


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10411
+->getAs();
+  if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return;
+

nickdesaulniers wrote:
> pirama wrote:
> > Add a comment explaining this conditional as well?  
> > 
> > > Return if source and target types are unavailable or if source is not a 
> > > floating point.
> > 
> > With the comment, it might be cleaner to read if you expand the negation: 
> > `!ResultBT || !RBT || !RBT->isFloatingPoint()`
> `ResultBT` and `RBT` are pointers that may be null and need to be checked.  
> The previous code did this before accessing them, I've just moved the checks 
> to be sooner.
> 
> If `!RBT` (if it's `nullptr`), then `!RBT->isFloatingPoint()` would be a null 
> deref, so unfortunately I can not expand it as recommended.
oops, nevermind


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 159820.
nickdesaulniers added a comment.

- clean up conditional and add comment


Repository:
  rC Clang

https://reviews.llvm.org/D50467

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-float-conversion.cpp

Index: test/SemaCXX/warn-float-conversion.cpp
===
--- test/SemaCXX/warn-float-conversion.cpp
+++ test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,22 @@
   l = ld;  //expected-warning{{conversion}}
 }
 
+void CompoundAssignment() {
+  int x = 3;
+
+  x += 1.234;  //expected-warning{{conversion}}
+  x -= -0.0;  //expected-warning{{conversion}}
+  x *= 1.1f;  //expected-warning{{conversion}}
+  x /= -2.2f;  //expected-warning{{conversion}}
+
+  int y = x += 1.4f;  //expected-warning{{conversion}}
+
+  float z = 1.1f;
+  double w = -2.2;
+
+  y += z + w;  //expected-warning{{conversion}}
+}
+
 void Test() {
   int a1 = 10.0/2.0;  //expected-warning{{conversion}}
   int a2 = 1.0/2.0;  //expected-warning{{conversion}}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10292,33 +10292,6 @@
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
-/// 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());
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // If both source and target are floating points.
-  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
-  // We don't want to warn for system macro.
-  if (!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);
-}
-
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
 SourceLocation CContext) {
@@ -10421,6 +10394,39 @@
   }
 }
 
+/// 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());
+
+  // 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 not.
+  if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+   E->getExprLoc());
+
+  // If both source and target are floating points.
+  // Builtin FP kinds are ordered by increasing FP rank.
+  if (ResultBT->getKind() < RBT->getKind() &&
+  // We don't want to warn for system macro.
+  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";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10424
+  // We don't want to warn for system macro.
+  S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.

aaron.ballman wrote:
> This looks incorrect to me -- this is testing the rank difference and that it 
> is in a system macro (the `!` was dropped). If this didn't cause any tests to 
> fail, we should add a test that would fail for it.
Thanks for the code review!

Sorry, I don't understand what you mean by "rank difference"? F16 vs F32 vs F64 
vs F128 etc?

When you say "this looks incorrect," are you commenting on the code I added 
(Lines 10415 to 10418) or the prexisting code that I moved (Lines 10420-10427)?

Good catch on dropping the `!`, that was definitely not intentional (and now 
I'm curious if `dw` in vim will delete `(!` together), will add back.

I'm happy to add a test for the system macro, but I also don't know what that 
is.  Can you explain?


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 160191.
nickdesaulniers added a comment.

- fix up system macro case and add test coverage for that case.


Repository:
  rC Clang

https://reviews.llvm.org/D50467

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-float-conversion.cpp

Index: test/SemaCXX/warn-float-conversion.cpp
===
--- test/SemaCXX/warn-float-conversion.cpp
+++ test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,32 @@
   l = ld;  //expected-warning{{conversion}}
 }
 
+void CompoundAssignment() {
+  int x = 3;
+
+  x += 1.234;  //expected-warning{{conversion}}
+  x -= -0.0;  //expected-warning{{conversion}}
+  x *= 1.1f;  //expected-warning{{conversion}}
+  x /= -2.2f;  //expected-warning{{conversion}}
+
+  int y = x += 1.4f;  //expected-warning{{conversion}}
+
+  float z = 1.1f;
+  double w = -2.2;
+
+  y += z + w;  //expected-warning{{conversion}}
+}
+
+# 1 "foo.h" 3
+//  ^ the following text comes from a system header file.
+#define SYSTEM_MACRO_FLOAT(x) do { (x) += 1.1; } while(0)
+# 1 "warn-float-conversion.cpp" 1
+//  ^ start of a new file.
+void SystemMacro() {
+  float x = 0.0f;
+  SYSTEM_MACRO_FLOAT(x);
+}
+
 void Test() {
   int a1 = 10.0/2.0;  //expected-warning{{conversion}}
   int a2 = 1.0/2.0;  //expected-warning{{conversion}}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10292,33 +10292,6 @@
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
-/// 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());
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // If both source and target are floating points.
-  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
-  // We don't want to warn for system macro.
-  if (!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);
-}
-
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
 SourceLocation CContext) {
@@ -10421,6 +10394,39 @@
   }
 }
 
+/// 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());
+
+  // 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 not.
+  if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+   E->getExprLoc());
+
+  // If both source and target are floating points.
+  // Builtin FP kinds are ordered by increasing FP rank.
+  if (ResultBT->getKind() < RBT->getKind() &&
+  // We don't want to warn for system macro.
+  !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";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for the info, I found 
https://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html helpful.  How does 
this look?


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339581: [SEMA] add more -Wfloat-conversion to compound 
assigment analysis (authored by nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D50467

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/SemaCXX/warn-float-conversion.cpp

Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -10282,33 +10282,6 @@
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
-/// 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());
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // If both source and target are floating points.
-  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
-  // We don't want to warn for system macro.
-  if (!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);
-}
-
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
 SourceLocation CContext) {
@@ -10411,6 +10384,39 @@
   }
 }
 
+/// 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());
+
+  // 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 not.
+  if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+   E->getExprLoc());
+
+  // If both source and target are floating points.
+  // Builtin FP kinds are ordered by increasing FP rank.
+  if (ResultBT->getKind() < RBT->getKind() &&
+  // We don't want to warn for system macro.
+  !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";
Index: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
===
--- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
+++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,32 @@
   l = ld;  //expected-warning{{conversion}}
 }
 
+void CompoundAssignment() {
+  int x = 3;
+
+  x += 1.234;  //expected-warning{{conversion}}
+  x -= -0.0;  //expected-warning{{conversion}}
+  x *= 1.1f;  //expected-warning{{conversion}}
+  x /= -2.2f;  //expected-warning{{conversion}}
+
+  int y = x += 1.4f;  //expected-warning{{conversion}}
+
+  float z = 1.1f;
+  double w = -2.2;
+
+  y += z + w;  //expected-warning{{conversion}}
+}
+
+# 1 "foo.h" 3
+//  ^ the following text comes from a system header file.
+#define SYSTEM_MACRO_FLOAT(x) do { (x) += 1.1; } while(0)
+# 1 "warn-float-conversion.cpp" 1
+//  ^ start of a new file.
+void SystemMacro() {
+  float x = 0.0f;
+  SYSTEM_MACRO_FLOAT(x);
+}
+
 void Test() {
   int a1 = 10.0/2.0;  //expected-warning{{conversion}}
   int a2 = 1.0/2.0;  //expected-warning{{conversion}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thank you for the code review.


Repository:
  rL LLVM

https://reviews.llvm.org/D50467



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

[PATCH] D50647: [Sema] fix -Wfloat-conversion test case.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: aaron.ballman, gkistanova.
Herald added a subscriber: cfe-commits.

Fixes commit 6bbde717f7fb ("[SEMA] add more -Wfloat-conversion to
compound assigment analysis").

This test case was caught in postsubmit testing.


Repository:
  rC Clang

https://reviews.llvm.org/D50647

Files:
  test/Sema/conversion.c


Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -359,7 +359,7 @@
 void test_7676608(void) {
   float q = 0.7f;
   char c = 5;
-  f7676608(c *= q);
+  f7676608(c *= q); // expected-warning {{conversion}}
 }
 
 // 


Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -359,7 +359,7 @@
 void test_7676608(void) {
   float q = 0.7f;
   char c = 5;
-  f7676608(c *= q);
+  f7676608(c *= q); // expected-warning {{conversion}}
 }
 
 // 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50647: [Sema] fix -Wfloat-conversion test case.

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339593: [Sema] fix -Wfloat-conversion test case. (authored 
by nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D50647

Files:
  cfe/trunk/test/Sema/conversion.c


Index: cfe/trunk/test/Sema/conversion.c
===
--- cfe/trunk/test/Sema/conversion.c
+++ cfe/trunk/test/Sema/conversion.c
@@ -359,7 +359,7 @@
 void test_7676608(void) {
   float q = 0.7f;
   char c = 5;
-  f7676608(c *= q);
+  f7676608(c *= q); // expected-warning {{conversion}}
 }
 
 // 


Index: cfe/trunk/test/Sema/conversion.c
===
--- cfe/trunk/test/Sema/conversion.c
+++ cfe/trunk/test/Sema/conversion.c
@@ -359,7 +359,7 @@
 void test_7676608(void) {
   float q = 0.7f;
   char c = 5;
-  f7676608(c *= q);
+  f7676608(c *= q); // expected-warning {{conversion}}
 }
 
 // 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50805: Don't warn on returning the address of a label from a statement expression

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thank you for this!


https://reviews.llvm.org/D50805



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

[PATCH] D51011: [Preprocessor] raise gcc compatibility macros.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Building the Linux kernel with clang is now broken as of commit
cafa0010cd51 ("Raise the minimum required gcc version to 4.6").

We were getting lucky that Clang previously declared compatibility with
gcc 4.2.1 as the kernel only errored out for gcc < 3.2. The compiler
check in the kernel should be improved as well.


Repository:
  rC Clang

https://reviews.llvm.org/D51011

Files:
  lib/Frontend/InitPreprocessor.cpp


Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -587,12 +587,12 @@
   "\"" CLANG_VERSION_STRING " "
   + getClangFullRepositoryVersion() + "\"");
   if (!LangOpts.MSVCCompat) {
-// Currently claim to be compatible with GCC 4.2.1-5621, but only if we're
+// Currently claim to be compatible with GCC 8.2, but only if we're
 // not compiling for MSVC compatibility
 Builder.defineMacro("__GNUC_MINOR__", "2");
-Builder.defineMacro("__GNUC_PATCHLEVEL__", "1");
-Builder.defineMacro("__GNUC__", "4");
-Builder.defineMacro("__GXX_ABI_VERSION", "1002");
+Builder.defineMacro("__GNUC_PATCHLEVEL__", "0");
+Builder.defineMacro("__GNUC__", "8");
+Builder.defineMacro("__GXX_ABI_VERSION", "1013");
   }
 
   // Define macros for the C11 / C++11 memory orderings


Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -587,12 +587,12 @@
   "\"" CLANG_VERSION_STRING " "
   + getClangFullRepositoryVersion() + "\"");
   if (!LangOpts.MSVCCompat) {
-// Currently claim to be compatible with GCC 4.2.1-5621, but only if we're
+// Currently claim to be compatible with GCC 8.2, but only if we're
 // not compiling for MSVC compatibility
 Builder.defineMacro("__GNUC_MINOR__", "2");
-Builder.defineMacro("__GNUC_PATCHLEVEL__", "1");
-Builder.defineMacro("__GNUC__", "4");
-Builder.defineMacro("__GXX_ABI_VERSION", "1002");
+Builder.defineMacro("__GNUC_PATCHLEVEL__", "0");
+Builder.defineMacro("__GNUC__", "8");
+Builder.defineMacro("__GXX_ABI_VERSION", "1013");
   }
 
   // Define macros for the C11 / C++11 memory orderings
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51011: [Preprocessor] raise gcc compatibility macros.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Richard,
Thanks for the review, context, and suggestions.  I appreciate it.

> Can you instead change the kernel to require GCC >= 4.6 or some suitable 
> version of Clang?

Can you help me create an accurate C preprocessor check that the compiler in 
use is actually GCC and not Clang or ICC in emulation?

  #if defined(__GNU_C__) && !defined(__clang__)
  // we're gcc
  #elif defined(__clang__)
  // we're clang
  #elif defined(__INTEL_COMPILER)
  // we're icc
  #endif

Or maybe:

  #ifdef(__clang__)
  // we're clang
  #elif defined(__INTEL_COMPILER)
  // we're icc
  #else
  // default to gcc
  #endif

Maybe GCC has something better for this case than `__GNU_C__`?

Regarding the glibc headers, do you know why glibc doesn't just add code for 
`__clang__` rather than Clang (and ICC) claim to be a gcc compatible to a point 
compiler?

Regarding the command line addition, I'd prefer to fix the kernel's compiler 
detection, but I will keep your idea in mind.  It's good to have a few 
solutions in our back pockets should the preferred be shown to be faulty.

Thanks again.


Repository:
  rC Clang

https://reviews.llvm.org/D51011



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

[PATCH] D51011: [Preprocessor] raise gcc compatibility macros.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks Eli.  I wholeheartedly prefer feature detection to explicit version 
checks.  One thing that makes this hard is the lack of __has_attribute in gcc: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970.


Repository:
  rC Clang

https://reviews.llvm.org/D51011



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

[PATCH] D51011: [Preprocessor] raise gcc compatibility macros.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Kernel patch: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=815f0ddb346c196018d4d8f8f55c12b83da1de3f

Thanks Eli and Richard, I appreciate it.


Repository:
  rC Clang

https://reviews.llvm.org/D51011



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

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.

This wasn't documented https://clang.llvm.org/docs/AttributeReference.html, and 
briefly mentioned 
https://clang.llvm.org/docs/UsersManual.html#differences-between-various-standard-modes.


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 162291.
nickdesaulniers added a comment.

- link to correct doc


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,19 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``inline`` should
+follow the c89 convention that if the function cannot be inlined (possibly due
+to being referred to by function pointer), then no out of line definition will
+be emitted (instead of c99's behaviour of always emitting an out of line
+definition). If ``__GNUC_STDC_INLINE__`` is defined, then the current
+translation unit is not being compiled with ``gnu_inline`` semantics, and the
+``gnu_inline`` function attribute can be used to get c89 semantics on a per
+function basis.  If ``__GNUC_STDC_INLINE__`` is not defined, then the
+translation unit is already being compiled with c89 semantics as the implied
+default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 162752.
nickdesaulniers added a comment.

- explicitly mention extern inline


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,20 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the c89 convention that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get c89 semantics on a per function basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with c89 semantics as the implied default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,20 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the c89 convention that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get c89 semantics on a per function basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with c89 semantics as the implied default.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 162761.
nickdesaulniers added a comment.

- s/c89/GNU inline extension/g and mention -std=gnu89/-fgnu89-inline


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,22 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the GNU inline extension that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function 
basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+This is the default behavior with ``-std=gnu89`` or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,22 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` attribute specifies that a function marked ``extern inline``
+should follow the GNU inline extension that if the function cannot be inlined
+(possibly due to being referred to by function pointer), then no out of line
+definition will be emitted (instead of c99's behaviour of always emitting an
+out of line definition).
+
+If ``__GNUC_STDC_INLINE__`` is defined, then the current translation unit is
+not being compiled with ``gnu_inline`` semantics, and the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function basis.
+If ``__GNUC_STDC_INLINE__`` is not defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+This is the default behavior with ``-std=gnu89`` or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 162908.
nickdesaulniers added a comment.

- Take rsmith's sugguested wording. Add info about __GNUC_STDC_INLINE__.


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,37 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint (note that this is different from
+the behavior of ``inline`` in both C99 inline semantics and C++ inline
+semantics).
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+And in particular as special cases, ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,37 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint (note that this is different from
+the behavior of ``inline`` in both C99 inline semantics and C++ inline
+semantics).
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+And in particular as special cases, ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1170,7 +1170,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 163215.
nickdesaulniers added a comment.

- link to correct doc
- explicitly mention extern inline
- s/c89/GNU inline extension/g and mention -std=gnu89/-fgnu89-inline
- Take rsmith's sugguested wording. Add info about __GNUC_STDC_INLINE__.
- some final touches recommended by rsmith


Repository:
  rC Clang

https://reviews.llvm.org/D51190

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,38 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint. In particular, an out-of-line
+definition is still emitted for a function with external linkage, even if all
+call sites are inlined, unlike in C99 and C++ inline semantics.
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+Some important consequences: ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default. It is
+unspecified which macro is defined in a C++ compilation.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1168,7 +1168,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,38 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint. In particular, an out-of-line
+definition is still emitted for a function with external linkage, even if all
+call sites are inlined, unlike in C99 and C++ inline semantics.
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+Some important consequences: ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default. It is
+unspecified which macro is defined in a C++ compilation.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/At

[PATCH] D51190: [AttrDocs]: document gnu_inline function attribute

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340987: [AttrDocs]: document gnu_inline function attribute 
(authored by nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51190

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td


Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,38 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint. In particular, an out-of-line
+definition is still emitted for a function with external linkage, even if all
+call sites are inlined, unlike in C99 and C++ inline semantics.
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+Some important consequences: ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default. It is
+unspecified which macro is defined in a C++ compilation.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -1168,7 +1168,7 @@
 def GNUInline : InheritableAttr {
   let Spellings = [GCC<"gnu_inline">];
   let Subjects = SubjectList<[Function]>;
-  let Documentation = [Undocumented];
+  let Documentation = [GnuInlineDocs];
 }
 
 def Hot : InheritableAttr {


Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -3505,3 +3505,38 @@
 invoking clang with -fno-c++-static-destructors.
   }];
 }
+
+def GnuInlineDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``gnu_inline`` changes the meaning of ``extern inline`` to use GNU inline
+semantics, meaning:
+
+* If any declaration that is declared ``inline`` is not declared ``extern``,
+then the ``inline`` keyword is just a hint. In particular, an out-of-line
+definition is still emitted for a function with external linkage, even if all
+call sites are inlined, unlike in C99 and C++ inline semantics.
+
+* If all declarations that are declared ``inline`` are also declared
+``extern``, then the function body is present only for inlining and no
+out-of-line version is emitted.
+
+Some important consequences: ``static inline`` emits an out-of-line
+version if needed, a plain ``inline`` definition emits an out-of-line version
+always, and an ``extern inline`` definition (in a header) followed by a
+(non-``extern``) ``inline`` declaration in a source file emits an out-of-line
+version of the function in that source file but provides the function body for
+inlining to all includers of the header.
+
+Either ``__GNUC_GNU_INLINE__`` (GNU inline semantics) or
+``__GNUC_STDC_INLINE__`` (C99 semantics) will be defined (they are mutually
+exclusive). If ``__GNUC_STDC_INLINE__`` is defined, then the ``gnu_inline``
+function attribute can be used to get GNU inline semantics on a per function
+basis. If ``__GNUC_GNU_INLINE__`` is defined, then the translation unit is
+already being compiled with GNU inline semantics as the implied default. It is
+unspecified which macro is defined in a C++ compilation.
+
+GNU inline semantics are the default behavior with ``-std=gnu89``,
+``-std=c89``, ``-std=c94``, or ``-fgnu89-inline``.
+  }];
+}
Index: cfe/trunk/include/clang/Basic/Attr.td
==

[PATCH] D48581: [AArch64] Support reserving x1-7 registers.

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for the other half of this feature!




Comment at: test/Driver/aarch64-fixed-x-register.c:35
+// RUN: FileCheck --check-prefix=CHECK-FIXED-X20 < %t %s
+// CHECK-FIXED-X20: "-target-feature" "+reserve-x20"

Is it worth checking a combination of these flags together work as expected 
(since I think you added tests that do that to llvm)?


https://reviews.llvm.org/D48581



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

[PATCH] D48581: [AArch64] Support reserving x1-7 registers.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

Great work Tri, thank you!


https://reviews.llvm.org/D48581



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

[PATCH] D49760: [clang:sema] de-duplicate getDepthAndIndex helpers

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, pirama.
Herald added a subscriber: cfe-commits.

Continuing off of:
https://reviews.llvm.org/D38382

Fixes:
https://bugs.llvm.org/show_bug.cgi?id=12176


Repository:
  rC Clang

https://reviews.llvm.org/D49760

Files:
  include/clang/Sema/SemaInternal.h
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateVariadic.cpp

Index: lib/Sema/SemaTemplateVariadic.cpp
===
--- lib/Sema/SemaTemplateVariadic.cpp
+++ lib/Sema/SemaTemplateVariadic.cpp
@@ -26,19 +26,6 @@
 // Visitor that collects unexpanded parameter packs
 //
 
-/// Retrieve the depth and index of a parameter pack.
-static std::pair 
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-  
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-  
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
 namespace {
   /// A class that collects unexpanded parameter packs.
   class CollectUnexpandedParameterPacksVisitor :
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -708,19 +708,6 @@
   return None;
 }
 
-/// Retrieve the depth and index of a parameter pack.
-static std::pair 
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-  
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-  
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
 //===--===/
 // Template Instantiation for Types
 //===--===/
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -628,29 +628,6 @@
   }
 }
 
-/// Retrieve the depth and index of a template parameter.
-static std::pair
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
-/// Retrieve the depth and index of an unexpanded parameter pack.
-static std::pair
-getDepthAndIndex(UnexpandedParameterPack UPP) {
-  if (const TemplateTypeParmType *TTP
-  = UPP.first.dyn_cast())
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-
-  return getDepthAndIndex(UPP.first.get());
-}
-
 /// Helper function to build a TemplateParameter when we don't
 /// know its type statically.
 static TemplateParameter makeTemplateParameter(Decl *D) {
Index: include/clang/Sema/SemaInternal.h
===
--- include/clang/Sema/SemaInternal.h
+++ include/clang/Sema/SemaInternal.h
@@ -101,6 +101,28 @@
   return nullptr;
 }
 
+/// Retrieve the depth and index of a template parameter.
+inline std::pair getDepthAndIndex(NamedDecl *ND) {
+  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
+return std::make_pair(TTP->getDepth(), TTP->getIndex());
+
+  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
+return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
+
+  TemplateTemplateParmDecl *TTP = cast(ND);
+  return std::make_pair(TTP->getDepth(), TTP->getIndex());
+}
+
+/// Retrieve the depth and index of an unexpanded parameter pack.
+inline std::pair
+getDepthAndIndex(UnexpandedParameterPack UPP) {
+  if (const TemplateTypeParmType *TTP =
+  UPP.first.dyn_cast())
+return std::make_pair(TTP->getDepth(), TTP->getIndex());
+
+  return getDepthAndIndex(UPP.first.get());
+}
+
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
   typedef SmallVector TypoResultList;
   typedef llvm::StringMap TypoResultsMap;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49760: [clang:sema] de-duplicate getDepthAndIndex helpers

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 157134.
nickdesaulniers added a comment.

- prefer auto when type is specified on RHS of assignment.


Repository:
  rC Clang

https://reviews.llvm.org/D49760

Files:
  include/clang/Sema/SemaInternal.h
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateVariadic.cpp

Index: lib/Sema/SemaTemplateVariadic.cpp
===
--- lib/Sema/SemaTemplateVariadic.cpp
+++ lib/Sema/SemaTemplateVariadic.cpp
@@ -26,19 +26,6 @@
 // Visitor that collects unexpanded parameter packs
 //
 
-/// Retrieve the depth and index of a parameter pack.
-static std::pair 
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-  
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-  
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
 namespace {
   /// A class that collects unexpanded parameter packs.
   class CollectUnexpandedParameterPacksVisitor :
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -708,19 +708,6 @@
   return None;
 }
 
-/// Retrieve the depth and index of a parameter pack.
-static std::pair 
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-  
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-  
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
 //===--===/
 // Template Instantiation for Types
 //===--===/
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -628,29 +628,6 @@
   }
 }
 
-/// Retrieve the depth and index of a template parameter.
-static std::pair
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
-/// Retrieve the depth and index of an unexpanded parameter pack.
-static std::pair
-getDepthAndIndex(UnexpandedParameterPack UPP) {
-  if (const TemplateTypeParmType *TTP
-  = UPP.first.dyn_cast())
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-
-  return getDepthAndIndex(UPP.first.get());
-}
-
 /// Helper function to build a TemplateParameter when we don't
 /// know its type statically.
 static TemplateParameter makeTemplateParameter(Decl *D) {
Index: include/clang/Sema/SemaInternal.h
===
--- include/clang/Sema/SemaInternal.h
+++ include/clang/Sema/SemaInternal.h
@@ -101,6 +101,27 @@
   return nullptr;
 }
 
+/// Retrieve the depth and index of a template parameter.
+inline std::pair getDepthAndIndex(NamedDecl *ND) {
+  if (const auto *TTP = dyn_cast(ND))
+return std::make_pair(TTP->getDepth(), TTP->getIndex());
+
+  if (const auto *NTTP = dyn_cast(ND))
+return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
+
+  const auto *TTP = cast(ND);
+  return std::make_pair(TTP->getDepth(), TTP->getIndex());
+}
+
+/// Retrieve the depth and index of an unexpanded parameter pack.
+inline std::pair
+getDepthAndIndex(UnexpandedParameterPack UPP) {
+  if (const auto *TTP = UPP.first.dyn_cast())
+return std::make_pair(TTP->getDepth(), TTP->getIndex());
+
+  return getDepthAndIndex(UPP.first.get());
+}
+
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
   typedef SmallVector TypoResultList;
   typedef llvm::StringMap TypoResultsMap;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49760: [clang:sema] de-duplicate getDepthAndIndex helpers

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for the review. Today is my first day committing to clang!


Repository:
  rC Clang

https://reviews.llvm.org/D49760



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

[PATCH] D49760: [clang:sema] de-duplicate getDepthAndIndex helpers

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337944: [clang:sema] de-duplicate getDepthAndIndex helpers 
(authored by nickdesaulniers, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D49760?vs=157134&id=157314#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49760

Files:
  include/clang/Sema/SemaInternal.h
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateVariadic.cpp

Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -708,19 +708,6 @@
   return None;
 }
 
-/// Retrieve the depth and index of a parameter pack.
-static std::pair 
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-  
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-  
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
 //===--===/
 // Template Instantiation for Types
 //===--===/
Index: lib/Sema/SemaTemplateVariadic.cpp
===
--- lib/Sema/SemaTemplateVariadic.cpp
+++ lib/Sema/SemaTemplateVariadic.cpp
@@ -26,19 +26,6 @@
 // Visitor that collects unexpanded parameter packs
 //
 
-/// Retrieve the depth and index of a parameter pack.
-static std::pair 
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-  
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-  
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
 namespace {
   /// A class that collects unexpanded parameter packs.
   class CollectUnexpandedParameterPacksVisitor :
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -628,29 +628,6 @@
   }
 }
 
-/// Retrieve the depth and index of a template parameter.
-static std::pair
-getDepthAndIndex(NamedDecl *ND) {
-  if (TemplateTypeParmDecl *TTP = dyn_cast(ND))
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-
-  if (NonTypeTemplateParmDecl *NTTP = dyn_cast(ND))
-return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
-
-  TemplateTemplateParmDecl *TTP = cast(ND);
-  return std::make_pair(TTP->getDepth(), TTP->getIndex());
-}
-
-/// Retrieve the depth and index of an unexpanded parameter pack.
-static std::pair
-getDepthAndIndex(UnexpandedParameterPack UPP) {
-  if (const TemplateTypeParmType *TTP
-  = UPP.first.dyn_cast())
-return std::make_pair(TTP->getDepth(), TTP->getIndex());
-
-  return getDepthAndIndex(UPP.first.get());
-}
-
 /// Helper function to build a TemplateParameter when we don't
 /// know its type statically.
 static TemplateParameter makeTemplateParameter(Decl *D) {
Index: include/clang/Sema/SemaInternal.h
===
--- include/clang/Sema/SemaInternal.h
+++ include/clang/Sema/SemaInternal.h
@@ -101,6 +101,27 @@
   return nullptr;
 }
 
+/// Retrieve the depth and index of a template parameter.
+inline std::pair getDepthAndIndex(NamedDecl *ND) {
+  if (const auto *TTP = dyn_cast(ND))
+return std::make_pair(TTP->getDepth(), TTP->getIndex());
+
+  if (const auto *NTTP = dyn_cast(ND))
+return std::make_pair(NTTP->getDepth(), NTTP->getIndex());
+
+  const auto *TTP = cast(ND);
+  return std::make_pair(TTP->getDepth(), TTP->getIndex());
+}
+
+/// Retrieve the depth and index of an unexpanded parameter pack.
+inline std::pair
+getDepthAndIndex(UnexpandedParameterPack UPP) {
+  if (const auto *TTP = UPP.first.dyn_cast())
+return std::make_pair(TTP->getDepth(), TTP->getIndex());
+
+  return getDepthAndIndex(UPP.first.get());
+}
+
 class TypoCorrectionConsumer : public VisibleDeclConsumer {
   typedef SmallVector TypoResultList;
   typedef llvm::StringMap TypoResultsMap;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

I think we should pass these flags along to the linker, too. Not just the 
assembler.  Maybe within tools::gnutools::Linker::ConstructJob() within 
lib/Driver/ToolChains/Gnu.cpp?


https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:543-549
+static const char* GetEndianArg(bool IsBigEndian, const ArgList &Args) {
+  if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+   options::OPT_mbig_endian)) {
+  IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
+  }
+  return (IsBigEndian) ? "-EB" : "-EL";
+}

If you passed `llvm::Triple` instead of `bool`, then I think you could do 
something like:

```
static const char* GetEndianArg(const llvm::Triple &triple, const ArgList 
&Args) {
  const bool IsBigEndian = triple == llvm::Triple::armeb ||
   triple == llvm::Triple::thumbeb ||
   triple == llvm::Triple::aarch64_be ||
   Args.getLastArg(options::OPT_mlittle_endian,
   
options::OPT_mbig_endian)->getOption().matches(options::OPT_mbig_endian);
  return IsBigEndian ? "-EB" : "-EL";
}
```

Might encapsulate the logic from the call sites better, but that's a minor nit.



Comment at: lib/Driver/ToolChains/Gnu.cpp:544-547
+  if (Arg *A = Args.getLastArg(options::OPT_mlittle_endian,
+   options::OPT_mbig_endian)) {
+  IsBigEndian = !A->getOption().matches(options::OPT_mlittle_endian);
+  }

Just so I understand this check, even if we didn't have a BE triple, we set 
`IsBigEndian` if `-mlittle-endian` was not set?  Is `-mlittle-endian` a default 
set flag, or does this set `"-EB"` even if no `-m*-endian` flag is specified (I 
think we want little-endian to be the default, and IIUC, this would change 
that)?


https://reviews.llvm.org/D52784



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

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344375: [Driver] check for exit code from SIGPIPE (authored 
by nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53001

Files:
  cfe/trunk/lib/Driver/Driver.cpp


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -78,6 +78,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include 
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
@@ -1388,20 +1389,28 @@
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-int Res = CmdPair.first;
+int CommandRes = CmdPair.first;
 const Command *FailingCommand = CmdPair.second;
 
 // Remove result files if we're not saving temps.
 if (!isSaveTempsEnabled()) {
   const JobAction *JA = cast(&FailingCommand->getSource());
   C.CleanupFileMap(C.getResultFiles(), JA, true);
 
   // Failure result files are valid unless we crashed.
-  if (Res < 0)
+  if (CommandRes < 0)
 C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
 }
 
+// llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+// for SIGPIPE. Do not print diagnostics for this case.
+if (CommandRes == EX_IOERR) {
+  Res = CommandRes;
+  continue;
+}
+
 // Print extra information about abnormal failures, if possible.
 //
 // This is ad-hoc, but we don't want to be excessively noisy. If the result
@@ -1411,17 +1420,17 @@
 // diagnostics, so always print the diagnostic there.
 const Tool &FailingTool = FailingCommand->getCreator();
 
-if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
+if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) 
{
   // FIXME: See FIXME above regarding result code interpretation.
-  if (Res < 0)
+  if (CommandRes < 0)
 Diag(clang::diag::err_drv_command_signalled)
 << FailingTool.getShortName();
   else
-Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-  << Res;
+Diag(clang::diag::err_drv_command_failed)
+<< FailingTool.getShortName() << CommandRes;
 }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -78,6 +78,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include 
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
@@ -1388,20 +1389,28 @@
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-int Res = CmdPair.first;
+int CommandRes = CmdPair.first;
 const Command *FailingCommand = CmdPair.second;
 
 // Remove result files if we're not saving temps.
 if (!isSaveTempsEnabled()) {
   const JobAction *JA = cast(&FailingCommand->getSource());
   C.CleanupFileMap(C.getResultFiles(), JA, true);
 
   // Failure result files are valid unless we crashed.
-  if (Res < 0)
+  if (CommandRes < 0)
 C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
 }
 
+// llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+// for SIGPIPE. Do not print diagnostics for this case.
+if (CommandRes == EX_IOERR) {
+  Res = CommandRes;
+  continue;
+}
+
 // Print extra information about abnormal failures, if possible.
 //
 // This is ad-hoc, but we don't want to be excessively noisy. If the result
@@ -1411,17 +1420,17 @@
 // diagnostics, so always print the diagnostic there.
 const Tool &FailingTool = FailingCommand->getCreator();
 
-if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
+if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) {
   // FIXME: See FIXME above regarding result code interpretation.
-  if (Res < 0)
+  if (CommandRes < 0)
 Diag(clang::diag::err_drv_command_signalled)
 << FailingTool.getShortName();
   else
-Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-  << Res;
+Diag(clang::diag::err_drv_command_failed)
+<< FailingTool.getShortName() << CommandRes;
 }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks for the addition of the flags to the linker. Interesting note about 
`-m*-endian` only being applicable for armv7.  Just some minor nits left.

With the current version of the patch, I can now assemble, link, and boot in a 
virtualized environment a big-endian armv8 Linux kernel with Clang. :)




Comment at: lib/Driver/ToolChains/Gnu.cpp:231
 
+// On Arm and endianness of the output file is determined by the target and
+// can be overridden by the pseudo-target flags '-mlittle-endian'/'-EL' and

> // On Arm and endianness
drop first `and`



Comment at: lib/Driver/ToolChains/Gnu.cpp:261-264
+if (isArmBigEndian(T, Args))
+  return "armelfb_linux_eabi";
+else
+  return "armelf_linux_eabi";

would a `?:` ternary fit on one line here?

> return isArmBigEndian(T, Args) ? "armelfb_linux_eabi" : "armelf_linux_eabi";



Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364
+const char* EndianFlag = "-EL";
+if (isArmBigEndian(Triple, Args)) {
+  EndianFlag = "-EB";
+  arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
+}
+else if (Arch == llvm::Triple::aarch64_be)
+  EndianFlag = "-EB";

```
bool IsBigEndian = isArmBigEndian(Triple, Args);
if (IsBigEndian)
  arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
IsBigEndian |= Arch == llvm::Triple::aarch64_be;
CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
```



Comment at: lib/Driver/ToolChains/Gnu.cpp:362
+}
+else if (Arch == llvm::Triple::aarch64_be)
+  EndianFlag = "-EB";

is having the `else if` on its own line what the formatter chose?



Comment at: lib/Driver/ToolChains/Gnu.cpp:667-670
+if (isArmBigEndian(Triple2, Args))
+  CmdArgs.push_back("-EB");
+else
+  CmdArgs.push_back("-EL");

Can we fit a ternary in one line here as well?

```
CmdArgs.push_back(isArmBigEndian(Triple2, Args) ? "-EB" : "-EL");
```



Comment at: lib/Driver/ToolChains/Gnu.cpp:703
   case llvm::Triple::aarch64_be: {
+if (getToolChain().getTriple().isLittleEndian())
+  CmdArgs.push_back("-EL");

earlier (L362), you check the endianess of the triple with:

```
Arch == llvm::Triple::aarch64_be
```
where `Arch` is `ToolChain.getArch()`.

I don't have a preference, but these two seem inconsistent.  Can we either 
check the explicit `llvm::Triple::` or call 
`getToolChain().getTriple().isLittleEndian()` in both, rather than mix?


https://reviews.llvm.org/D52784



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Driver/ToolChains/Gnu.cpp:357-364
+const char* EndianFlag = "-EL";
+if (isArmBigEndian(Triple, Args)) {
+  EndianFlag = "-EB";
+  arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
+}
+else if (Arch == llvm::Triple::aarch64_be)
+  EndianFlag = "-EB";

nickdesaulniers wrote:
> ```
> bool IsBigEndian = isArmBigEndian(Triple, Args);
> if (IsBigEndian)
>   arm::appendBE8LinkFlag(Args, CmdArgs, Triple);
> IsBigEndian |= Arch == llvm::Triple::aarch64_be;
> CmdArgs.push_back(IsBigEndian ? "-EB" : "-EL");
> ```
`IsBigEndian |= Arch == llvm::Triple::aarch64_be;`

should be:

`IsBigEndian = IsBigEndian || Arch == llvm::Triple::aarch64_be;`

in order to not evaluate `Arch == llvm::Triple::aarch64_b` if `IsBigEndian` is 
already true.


https://reviews.llvm.org/D52784



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

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

@rnk has reverted this due to breaking the windows build (sysexits.h) is not 
available there.


Repository:
  rL LLVM

https://reviews.llvm.org/D53001



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

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: cfe/trunk/lib/Driver/Driver.cpp:81-85
+#include 
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
 #endif

@rnk || @majnemer thoughts on making this:

```
#if LLVM_ON_UNIX
​#include  // getpid
#include  // EX_IOERR
#else
#define EX_IOERR 74
​#endif
```

vs also guarding the check I added on L1409 with `#if LLVM_ON_UNIX`?


Repository:
  rL LLVM

https://reviews.llvm.org/D53001



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

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: rnk, majnemer, jfb.
Herald added a subscriber: cfe-commits.

Add preprocessor guards for UNIX.

This reverts commit c2275676c1bde4ed20c06affdeeb1a93692f29a8.


Repository:
  rC Clang

https://reviews.llvm.org/D53210

Files:
  lib/Driver/Driver.cpp


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -81,6 +81,7 @@
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
+#include  // EX_IOERR
 #endif
 
 using namespace clang::driver;
@@ -1388,20 +1389,30 @@
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-int Res = CmdPair.first;
+int CommandRes = CmdPair.first;
 const Command *FailingCommand = CmdPair.second;
 
 // Remove result files if we're not saving temps.
 if (!isSaveTempsEnabled()) {
   const JobAction *JA = cast(&FailingCommand->getSource());
   C.CleanupFileMap(C.getResultFiles(), JA, true);
 
   // Failure result files are valid unless we crashed.
-  if (Res < 0)
+  if (CommandRes < 0)
 C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
 }
 
+#if LLVM_ON_UNIX
+// llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+// for SIGPIPE. Do not print diagnostics for this case.
+if (CommandRes == EX_IOERR) {
+  Res = CommandRes;
+  continue;
+}
+#endif
+
 // Print extra information about abnormal failures, if possible.
 //
 // This is ad-hoc, but we don't want to be excessively noisy. If the result
@@ -1411,17 +1422,17 @@
 // diagnostics, so always print the diagnostic there.
 const Tool &FailingTool = FailingCommand->getCreator();
 
-if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
+if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) 
{
   // FIXME: See FIXME above regarding result code interpretation.
-  if (Res < 0)
+  if (CommandRes < 0)
 Diag(clang::diag::err_drv_command_signalled)
 << FailingTool.getShortName();
   else
-Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-  << Res;
+Diag(clang::diag::err_drv_command_failed)
+<< FailingTool.getShortName() << CommandRes;
 }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -81,6 +81,7 @@
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
+#include  // EX_IOERR
 #endif
 
 using namespace clang::driver;
@@ -1388,20 +1389,30 @@
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-int Res = CmdPair.first;
+int CommandRes = CmdPair.first;
 const Command *FailingCommand = CmdPair.second;
 
 // Remove result files if we're not saving temps.
 if (!isSaveTempsEnabled()) {
   const JobAction *JA = cast(&FailingCommand->getSource());
   C.CleanupFileMap(C.getResultFiles(), JA, true);
 
   // Failure result files are valid unless we crashed.
-  if (Res < 0)
+  if (CommandRes < 0)
 C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
 }
 
+#if LLVM_ON_UNIX
+// llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+// for SIGPIPE. Do not print diagnostics for this case.
+if (CommandRes == EX_IOERR) {
+  Res = CommandRes;
+  continue;
+}
+#endif
+
 // Print extra information about abnormal failures, if possible.
 //
 // This is ad-hoc, but we don't want to be excessively noisy. If the result
@@ -1411,17 +1422,17 @@
 // diagnostics, so always print the diagnostic there.
 const Tool &FailingTool = FailingCommand->getCreator();
 
-if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
+if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) {
   // FIXME: See FIXME above regarding result code interpretation.
-  if (Res < 0)
+  if (CommandRes < 0)
 Diag(clang::diag::err_drv_command_signalled)
 << FailingTool.getShortName();
   else
-Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-  << Res;
+Diag(clang::diag::err_drv_command_failed)
+<< FailingTool.getShortName() << CommandRes;
 }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D53001: [Driver] check for exit code from SIGPIPE

[Nick Desaulniers via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/Driver/Driver.cpp:81-85
+#include 
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
 #endif

nickdesaulniers wrote:
> @rnk || @majnemer thoughts on making this:
> 
> ```
> #if LLVM_ON_UNIX
> ​#include  // getpid
> #include  // EX_IOERR
> #else
> #define EX_IOERR 74
> ​#endif
> ```
> 
> vs also guarding the check I added on L1409 with `#if LLVM_ON_UNIX`?
posted to: https://reviews.llvm.org/D53210
(let's do code review there).


Repository:
  rL LLVM

https://reviews.llvm.org/D53001



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

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344536: Revert 344389 "Revert r344375 "[Driver] 
check for exit code from SIGPIPE"" (authored by nickdesaulniers, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53210?vs=169474&id=169723#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53210

Files:
  lib/Driver/Driver.cpp


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -81,6 +81,7 @@
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
+#include  // EX_IOERR
 #endif
 
 using namespace clang::driver;
@@ -1388,20 +1389,30 @@
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-int Res = CmdPair.first;
+int CommandRes = CmdPair.first;
 const Command *FailingCommand = CmdPair.second;
 
 // Remove result files if we're not saving temps.
 if (!isSaveTempsEnabled()) {
   const JobAction *JA = cast(&FailingCommand->getSource());
   C.CleanupFileMap(C.getResultFiles(), JA, true);
 
   // Failure result files are valid unless we crashed.
-  if (Res < 0)
+  if (CommandRes < 0)
 C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
 }
 
+#if LLVM_ON_UNIX
+// llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+// for SIGPIPE. Do not print diagnostics for this case.
+if (CommandRes == EX_IOERR) {
+  Res = CommandRes;
+  continue;
+}
+#endif
+
 // Print extra information about abnormal failures, if possible.
 //
 // This is ad-hoc, but we don't want to be excessively noisy. If the result
@@ -1411,17 +1422,17 @@
 // diagnostics, so always print the diagnostic there.
 const Tool &FailingTool = FailingCommand->getCreator();
 
-if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
+if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) 
{
   // FIXME: See FIXME above regarding result code interpretation.
-  if (Res < 0)
+  if (CommandRes < 0)
 Diag(clang::diag::err_drv_command_signalled)
 << FailingTool.getShortName();
   else
-Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-  << Res;
+Diag(clang::diag::err_drv_command_failed)
+<< FailingTool.getShortName() << CommandRes;
 }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -81,6 +81,7 @@
 #include 
 #if LLVM_ON_UNIX
 #include  // getpid
+#include  // EX_IOERR
 #endif
 
 using namespace clang::driver;
@@ -1388,20 +1389,30 @@
 
   // Otherwise, remove result files and print extra information about abnormal
   // failures.
+  int Res = 0;
   for (const auto &CmdPair : FailingCommands) {
-int Res = CmdPair.first;
+int CommandRes = CmdPair.first;
 const Command *FailingCommand = CmdPair.second;
 
 // Remove result files if we're not saving temps.
 if (!isSaveTempsEnabled()) {
   const JobAction *JA = cast(&FailingCommand->getSource());
   C.CleanupFileMap(C.getResultFiles(), JA, true);
 
   // Failure result files are valid unless we crashed.
-  if (Res < 0)
+  if (CommandRes < 0)
 C.CleanupFileMap(C.getFailureResultFiles(), JA, true);
 }
 
+#if LLVM_ON_UNIX
+// llvm/lib/Support/Unix/Signals.inc will exit with a special return code
+// for SIGPIPE. Do not print diagnostics for this case.
+if (CommandRes == EX_IOERR) {
+  Res = CommandRes;
+  continue;
+}
+#endif
+
 // Print extra information about abnormal failures, if possible.
 //
 // This is ad-hoc, but we don't want to be excessively noisy. If the result
@@ -1411,17 +1422,17 @@
 // diagnostics, so always print the diagnostic there.
 const Tool &FailingTool = FailingCommand->getCreator();
 
-if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) {
+if (!FailingCommand->getCreator().hasGoodDiagnostics() || CommandRes != 1) {
   // FIXME: See FIXME above regarding result code interpretation.
-  if (Res < 0)
+  if (CommandRes < 0)
 Diag(clang::diag::err_drv_command_signalled)
 << FailingTool.getShortName();
   else
-Diag(clang::diag::err_drv_command_failed) << FailingTool.getShortName()
-  << Res;
+Diag(clang::diag::err_drv_command_failed)
+<< FailingTool.getShortName() << CommandRes;
 }
   }
-  return 0;
+  return Res;
 }
 
 void Driver::PrintHelp(bool ShowHidden) const {
___
cfe-commits mai

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for this patch.  With it I was able to link+boot a BE aarch64 Linux 
kernel (and a LE aarch64 Linux kernel).




Comment at: lib/Driver/ToolChains/Gnu.cpp:268
   case llvm::Triple::thumbeb:
-return "armelfb_linux_eabi";
+return (isArmBigEndian(T, Args)) ? "armelfb_linux_eabi"
+ : "armelf_linux_eabi";

probably don't need the parens around `isArmBigEndian(...)`.


https://reviews.llvm.org/D52784



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

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

I think this is now breaking:
lld :: ELF/format-binary.test
lld :: ELF/relocatable-versioned.s


Repository:
  rC Clang

https://reviews.llvm.org/D53210



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

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Nevermind, looks like flaky tests.  Will try to repro and contact msan 
maintainers.


Repository:
  rC Clang

https://reviews.llvm.org/D53210



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

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks again for this patch!


Repository:
  rC Clang

https://reviews.llvm.org/D52784



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

[PATCH] D53463: [Driver] allow Android triples to alias for non Android targets

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, pirama, danalbert.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.

Partial revert of r330873 ('[Driver] Reland "Android triples are not
aliases for other triples."')

While we don't want `-target aarch64-linux-android` to alias to non
*-android libs and binaries, it turns out we do want the opposite. Ie. We
would like for `-target aarch64-linux-gnu` to still be able to use
*-android libs and binaries.

In fact, this is used to cross assemble and link the Linux kernel for
Android devices.

`-target aarch64-linux-gnu` needs to be used for the Linux kernel when
using the android binutils prebuilts.

The use of `-target aarch64-linux-android` on C source files will cause
Clang to perform optimizations based on the presence of bionic (due to
r265481 ('Faster stack-protector for Android/AArch64.')) which is
invalid within the Linux kernel and will produce a non-bootable kernel
image.

Of course, you could just use the standard binutils (aarch64-linux-gnu),
but Android does not distribute these.  So this patch fixes a problem
that only occurs when cross assembling and linking a Linux kernel with
the Android provided binutils, which is what is done within Android's
hermetic build system.

Fixes b/117673837.


Repository:
  rC Clang

https://reviews.llvm.org/D53463

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/android-gcc-toolchain.c


Index: test/Driver/android-gcc-toolchain.c
===
--- /dev/null
+++ test/Driver/android-gcc-toolchain.c
@@ -0,0 +1,8 @@
+// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
+// triple.
+//
+// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
+// RUN: | FileCheck %s
+//
+// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1838,19 +1838,21 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux"};
+  "aarch64-suse-linux", "aarch64-linux-android"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
+   "arm-linux-androideabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
+ "armeb-linux-androideabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -1861,22 +1863,24 @@
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux"};
+  "x86_64-amazon-linux","x86_64-linux-android"};
   static const char *const X32LibDirs[] = {"/libx32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
   "i686-linux-gnu",   "i686-pc-linux-gnu", "i486-linux-gnu",
   "i386-linux-gnu",   "i386-redhat-linux6E",   "i686-redhat-linux",
   "i586-redhat-linux","i386-redhat-linux", "i586-suse-linux",
-  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu"};
+  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu",
+  "i686-linux-android"};
 
   static const char *const MIPSLibDirs[] = {"/lib"};
   static const char *const MIPSTriples[] = {
   "mips-linux-gnu", "mips-mti-linux", "mips-mti-linux-gnu",
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
+  "mipsel-linux-android"};
 
   static

[PATCH] D53463: [Driver] allow Android triples to alias for non Android targets

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344941: [Driver] allow Android triples to alias for non 
Android targets (authored by nickdesaulniers, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53463?vs=170283&id=170478#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53463

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/android-gcc-toolchain.c


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -0,0 +1,8 @@
+// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
+// triple.
+//
+// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
+// RUN: | FileCheck %s
+//
+// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1847,19 +1847,21 @@
   static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const AArch64Triples[] = {
   "aarch64-none-linux-gnu", "aarch64-linux-gnu", "aarch64-redhat-linux",
-  "aarch64-suse-linux"};
+  "aarch64-suse-linux", "aarch64-linux-android"};
   static const char *const AArch64beLibDirs[] = {"/lib"};
   static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu",
  "aarch64_be-linux-gnu"};
 
   static const char *const ARMLibDirs[] = {"/lib"};
-  static const char *const ARMTriples[] = {"arm-linux-gnueabi"};
+  static const char *const ARMTriples[] = {"arm-linux-gnueabi",
+   "arm-linux-androideabi"};
   static const char *const ARMHFTriples[] = {"arm-linux-gnueabihf",
  "armv7hl-redhat-linux-gnueabi",
  "armv6hl-suse-linux-gnueabi",
  "armv7hl-suse-linux-gnueabi"};
   static const char *const ARMebLibDirs[] = {"/lib"};
-  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi"};
+  static const char *const ARMebTriples[] = {"armeb-linux-gnueabi",
+ "armeb-linux-androideabi"};
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
@@ -1870,22 +1872,24 @@
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
   "x86_64-slackware-linux", "x86_64-unknown-linux",
-  "x86_64-amazon-linux"};
+  "x86_64-amazon-linux","x86_64-linux-android"};
   static const char *const X32LibDirs[] = {"/libx32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
   "i686-linux-gnu",   "i686-pc-linux-gnu", "i486-linux-gnu",
   "i386-linux-gnu",   "i386-redhat-linux6E",   "i686-redhat-linux",
   "i586-redhat-linux","i386-redhat-linux", "i586-suse-linux",
-  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu"};
+  "i486-slackware-linux", "i686-montavista-linux", "i586-linux-gnu",
+  "i686-linux-android"};
 
   static const char *const MIPSLibDirs[] = {"/lib"};
   static const char *const MIPSTriples[] = {
   "mips-linux-gnu", "mips-mti-linux", "mips-mti-linux-gnu",
   "mips-img-linux-gnu", "mipsisa32r6-linux-gnu"};
   static const char *const MIPSELLibDirs[] = {"/lib"};
   static const char *const MIPSELTriples[] = {
-  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu"};
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
+  "mipsel-linux-android"};
 
   static const char *const MIPS64LibDirs[] = {"/lib64", "/lib"};
   static const char *const MIPS64Triples[] = {
@@ -1896,7 +1900,8 @@
   static const char *const MIPS64ELTriples[] = {
   "mips64el-linux-gnu",  "mips-mti-linux-gnu",
   "mips-img-linux-gnu",  "mips64el-linux-gnuabi64",
-  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64"};
+  "mipsisa64r6el-linux-gnu", "mipsisa64r6el-linux-gnuabi64",
+  "mips64el-linux-android"};
 
   static const char *const MIPSN32LibDirs[] = {"/lib32"};
   static const char *const MIPSN32Triples[] = {"mips64-linux-gnuabin32",


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -0,0 +1,8 @@
+// Test that gcc-toolchain option works correctly with a aarch64-linux-gnu
+// triple.
+//
+// RUN: %clang %s -### -v --target=aarch64-linux-gnu \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_and

[PATCH] D53529: [Driver] fix broken test

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, danalbert.
Herald added a subscriber: cfe-commits.

Fixes test from r344941 which was broken on Windows. We want to check
the selected toolchain rather than the found toolchain anyways.


Repository:
  rC Clang

https://reviews.llvm.org/D53529

Files:
  test/Driver/android-gcc-toolchain.c


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9


Index: test/Driver/android-gcc-toolchain.c
===
--- test/Driver/android-gcc-toolchain.c
+++ test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53529: [Driver] fix broken test

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344958: [Driver] fix broken test (authored by 
nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53529

Files:
  cfe/trunk/test/Driver/android-gcc-toolchain.c


Index: cfe/trunk/test/Driver/android-gcc-toolchain.c
===
--- cfe/trunk/test/Driver/android-gcc-toolchain.c
+++ cfe/trunk/test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: 
{{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9


Index: cfe/trunk/test/Driver/android-gcc-toolchain.c
===
--- cfe/trunk/test/Driver/android-gcc-toolchain.c
+++ cfe/trunk/test/Driver/android-gcc-toolchain.c
@@ -5,4 +5,4 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_android_ndk_tree/ 2>&1 \
 // RUN: | FileCheck %s
 //
-// CHECK: Found candidate GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
+// CHECK: Selected GCC installation: {{.*}}/Inputs/basic_android_ndk_tree/lib/gcc/aarch64-linux-android/4.9
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks for all the work that went into this patch, both initially and rebasing!




Comment at: lib/AST/Stmt.cpp:465
+  const LabelDecl *LD = getLabelExpr(i)->getLabel();
+  return LD->getName();
+}

These 2 statements can probably be condensed into one.
```
return getLabelExpr(i)->getLabel()->getName();
```



Comment at: lib/CodeGen/CGStmt.cpp:2182
+}
+  }
+

If this new block was moved closer to the new one on L2227, I assume they could 
be combined and possibly `IsGCCAsmGoto` be removed?  The code currently in 
between doesn't appear at first site to depend on info from this block, though 
maybe I may be missing it.



Comment at: lib/Parse/ParseStmtAsm.cpp:830-858
+  if (AteExtraColon || Tok.is(tok::colon)) {
+if (AteExtraColon)
+  AteExtraColon = false;
+else
+  ConsumeToken();
+
+if (!AteExtraColon && Tok.isNot(tok::identifier)) {

```
if (x || y) {
  if (x) foo();
  else bar();
  if (!x && ...) baz();
  if (!x && ...) quux();
```
is maybe more readable as:
```
if (x) foo();
else if (y)
  bar();
  baz();
  quux();
```



Comment at: test/CodeGen/asm.c:271
+  // CHECK: callbr void asm sideeffect "testl $0, $0; jne ${1:l};", 
"r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@t32, 
%label_true), i8* blockaddress(@t32, %loop)) #1
+return 0;
+loop:

extra indentation?



Comment at: test/Parser/asm.c:22-37
+// expected-error@+1 {{expected ')'}}
+asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+// expected-error@+1 {{expected ':'}}
+asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+// expected-error@+1 {{expected identifie}}
+asm goto ("decl %0;" :: "m"(x) : "memory" : );
+// expected-error@+1 {{expected ':'}} 

Extra indentation intentional?



Comment at: test/Parser/asm.cpp:14-29
+// expected-error@+1 {{expected ')'}}
+asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+// expected-error@+1  {{expected ':'}}
+asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+// expected-error@+1 {{expected identifie}}
+asm goto ("decl %0;" :: "m"(x) : "memory" : );
+// expected-error@+1  {{expected ':'}}

ditto


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:830-858
+  if (AteExtraColon || Tok.is(tok::colon)) {
+if (AteExtraColon)
+  AteExtraColon = false;
+else
+  ConsumeToken();
+
+if (!AteExtraColon && Tok.isNot(tok::identifier)) {

jyu2 wrote:
> nickdesaulniers wrote:
> > ```
> > if (x || y) {
> >   if (x) foo();
> >   else bar();
> >   if (!x && ...) baz();
> >   if (!x && ...) quux();
> > ```
> > is maybe more readable as:
> > ```
> > if (x) foo();
> > else if (y)
> >   bar();
> >   baz();
> >   quux();
> > ```
> This is better?
IIUC, it looks like `ConsumeToken()` modifies `Tok`, so the 2 `isNot` cases 
should fail as expected when `!AteExtraColon`, so this change LGTM.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

With this patch, DD53765, and one out of tree kernel patch to work around a 
separate issue 
,
 I can confirm that I can compile, link, and boot an x86_64 Linux kernel with 
Clang. :)  I'm triple checking with a few other architectures (that also suffer 
from the separate issue) to see if I can find any codegen issues.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Also of note: glib, a super popular library used across many Linux distro's 
will benefit from the implementation of asm goto (I will send them a patch to 
fix their version detection once we land in Clang): 
https://github.com/GNOME/glib/blob/1ba843b8a0585f20438d5617521f247071c6d94c/glib/gbitlock.c#L181


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

https://reviews.llvm.org/D56571



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

[PATCH] D53921: Compound literals and enums require constant inits

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:16411
 // this constant.  Skip this enum since it may be ill-formed.
-if (!ECD) {
-  return;
-}
+if (!ECD) return;
 

probably don't need to adjust this line


Repository:
  rC Clang

https://reviews.llvm.org/D53921



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

[PATCH] D54188: [Sema] Mark target of __attribute__((alias("target"))) used for C

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: rsmith.
Herald added subscribers: cfe-commits, erik.pilkington.

Prevents -Wunneeded-internal-delcaration warnings when the target has no
other references. This occurs frequently in device drivers in the Linux
kernel.

Sema would need to invoke the demangler on the target, since in C++ the
target name is mangled:

int f() { return 42; }
int g() __attribute__((alias("_Z1fv")));

Sema does not have the ability to demangle names at this time.

https://bugs.llvm.org/show_bug.cgi?id=39088
ClangBuiltLinux/linux#203
ClangBuiltLinux/linux#205
ClangBuiltLinux/linux#207
ClangBuiltLinux/linux#209
ClangBuiltLinux/linux#232


Repository:
  rC Clang

https://reviews.llvm.org/D54188

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/alias-unused.c


Index: test/Sema/alias-unused.c
===
--- /dev/null
+++ test/Sema/alias-unused.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x 
c -verify %s
+// expected-no-diagnostics
+int f() { return 42; }
+int g() __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1851,7 +1851,16 @@
 }
   }
 
-  // FIXME: check if target symbol exists in current file
+  // Mark target used to prevent unneeded-internal-declaration warnings.
+  if (!S.LangOpts.CPlusPlus) {
+// FIXME: demangle Str for C++, as the attribute refers to the mangled
+// linkage name, not the pre-mangled identifier.
+const DeclarationNameInfo target(&S.Context.Idents.get(Str), AL.getLoc());
+LookupResult LR(S, target, Sema::LookupOrdinaryName);
+if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+  for (NamedDecl *ND : LR)
+ND->markUsed(S.Context);
+  }
 
   D->addAttr(::new (S.Context) AliasAttr(AL.getRange(), S.Context, Str,
  AL.getAttributeSpellingListIndex()));


Index: test/Sema/alias-unused.c
===
--- /dev/null
+++ test/Sema/alias-unused.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -Wunneeded-internal-declaration -x c -verify %s
+// expected-no-diagnostics
+int f() { return 42; }
+int g() __attribute__((alias("f")));
+
+static int foo [] = { 42, 0xDEAD };
+extern typeof(foo) bar __attribute__((unused, alias("foo")));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1851,7 +1851,16 @@
 }
   }
 
-  // FIXME: check if target symbol exists in current file
+  // Mark target used to prevent unneeded-internal-declaration warnings.
+  if (!S.LangOpts.CPlusPlus) {
+// FIXME: demangle Str for C++, as the attribute refers to the mangled
+// linkage name, not the pre-mangled identifier.
+const DeclarationNameInfo target(&S.Context.Idents.get(Str), AL.getLoc());
+LookupResult LR(S, target, Sema::LookupOrdinaryName);
+if (S.LookupQualifiedName(LR, S.getCurLexicalContext()))
+  for (NamedDecl *ND : LR)
+ND->markUsed(S.Context);
+  }
 
   D->addAttr(::new (S.Context) AliasAttr(AL.getRange(), S.Context, Str,
  AL.getAttributeSpellingListIndex()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1285
 
 case Expr::ConstantExprClass:
 case Stmt::ExprWithCleanupsClass:

`LLVM_FALLTHROUGH;` ?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1285
 
 case Expr::ConstantExprClass:
 case Stmt::ExprWithCleanupsClass:

nickdesaulniers wrote:
> `LLVM_FALLTHROUGH;` ?
nvm, only needed when there's code THEN fallthrough.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/AST/ExprConstant.cpp:11391
 
   llvm_unreachable("Invalid StmtClass!");
 }

@void , I assume this unreachable is the one reported by @uweigand ? Does the 
above switch need a case statement added?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D53210: Revert 344389 "Revert r344375 "[Driver] check for exit code from SIGPIPE""

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Hi @richard.barton.arm , this has landed already.  If you click "show older" 
you'll see the UI element that shows this landed as r344536 (Oct 15 2018).  If 
your clang version is later than r344536 (what will become clang-8) and you 
still see this, please open a new bug.  The commit is definitely confusing; it 
looks like a revert, but it's a revert of a revert (double negative) which is 
relanding the patch.  The lld failures were PEBKAC on my part, unrelated to 
this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D53210



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

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Bump, this is still listed as a TODO in the Linux kernel that works around the 
issue.


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

https://reviews.llvm.org/D38479



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Should the langref be updated (specifically the section on blockaddress):

  This value only has defined behavior when used as an operand to the 
‘indirectbr’ instruction, or for comparisons against null.

https://reviews.llvm.org/D53765 touched the langref, but I think the 
blockaddress section can be cleaned up.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

needs another rebase


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

> which is making things hard for the folks testing this with the Linux kernel 
> and needing to apply this patch themselves.

If we can get inlining of callbr working first before landing this, then all of 
our CI won't go immediately red (as it would from landing this first, then 
getting inlining support).  Otherwise, we'll have to come up with hacks in the 
kernel to keep the CI green (I have one for x86.  The one for aarch64 quickly 
became infeasible).

I'm anxious for this to land, and the rebases are work indeed.  But if we can 
just wait a little longer and get inlining support working, then land this, it 
will be much less painful than the other way around.

I'm focused on inlining support in https://reviews.llvm.org/D58260 (which 
doesn't look like much now, but I have changes locally and have been actively 
working on it this week).


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

https://reviews.llvm.org/D56571



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

This is causing false positive warnings for the Linux kernel:
https://github.com/ClangBuiltLinux/linux/issues/423
:(

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/statfs.c#n128
Consider this untested case:

if (sizeof(buf) == sizeof(*st))
memcpy(&buf, st, sizeof(*st));

fs/statfs.c:129:3: warning: 'memcpy' will always overflow; destination buffer 
has size 64, but size argument is 88 [-Wfortify-source]

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/statfs.c#n169,
 too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D58797#1439888 , @erik.pilkington 
wrote:

> Ah, I didn't consider that case. Presumably `st` is configured to have 
> different sizes based on the target?


Yes; sorry I was not clear about that in my example.

> I agree that this is a false-positive, but it seems like a pretty narrow edge 
> case, and there is a pretty obvious source workaround that doesn't affect 
> readability: `memcpy(&buf, st, sizeof(buf))`.

Oh, yeah, we could make that source level change.

> @aaron.ballman/@rsmith Any thoughts here? IMO keeping this diagnostic is 
> worth it.

I'm all for keeping it, I'm curious if it can be extended to NOT warn in the 
case provided?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D58797#1443856 , @erik.pilkington 
wrote:

> I added it in r357041.


LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D60059: [Driver] implement -feverything

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: rsmith.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert.
Herald added projects: clang, LLVM.

Due to the wild popularity of -Weverything (used for enabling all
warnings), we're back for round 2 (electric boogaloo) with a succinct
analog to enable all -f flags; -feverything.

This includes favorite hits like:

- -funsafe-math-optimizations
  - who knew floating point math could be BOTH fun and safe, WHO KNEW?!1
- your favorite language extensions
  - -fborland-extensions
  - -fms-extensions
  - -fheinous-gnu-extensions
  - -fplan9-extensions (TODO: implement like GCC)
- flags specific to languages (whether you're using them or not) like:
  - CUDA
  - FORTRAN
  - Objective-C
  - OpenMP
- flags specific to target (whether you're targeting them or not) like:
  - -faltivec
  - -fcray-pointer
  - -fcall-saved-*/-ffixed-* (aka "build your own calling convention!")
- I don't know what these do, but they sound nice:
  - -fmudflap
  - -fdollar-ok
  - -fropi
  - -frwpi

Can't decide between the overflow behavior of -fwrapv or -ftrapv?
¿Porque No Los Dos?

Next time you're having trouble getting your code to compile, try
-feverything.

TODO:

- unit tests (honestly afraid to run this outside a VM, though this does

enable all runtime sanitizers for extra safety).

- -fno-everything


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60059

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/Option/OptTable.h


Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -88,6 +88,8 @@
   /// Return the total number of option classes.
   unsigned getNumOptions() const { return OptionInfos.size(); }
 
+  unsigned getFirstSearchableIndex() const { return FirstSearchableIndex; }
+
   /// Get the given Opt's Option instance, lazily creating it
   /// if necessary.
   ///
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1427,6 +1427,25 @@
   return Success;
 }
 
+void clang::ParseFEverything(llvm::opt::OptTable *Opts,
+ llvm::opt::InputArgList *Args) {
+  if (!Args->hasArg(options::OPT_feverything))
+return;
+  const unsigned f_group = Opts->getOption(OPT_f_Group).getID();
+  unsigned i = Opts->getFirstSearchableIndex();
+  const unsigned len = Opts->getNumOptions();
+  for (; i != len; ++i) {
+const Option &opt = Opts->getOption(i);
+const Option group = opt.getGroup();
+// TODO: add more groups
+if (group.isValid() && group.getID() == f_group) {
+  const std::string Flag = opt.getPrefixedName();
+  if (Flag.back() != '=' && Flag.back() != '-')
+Args->append(new Arg(opt, Flag, Args->MakeIndex(Flag), Flag.c_str()));
+}
+  }
+}
+
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
 DiagnosticsEngine *Diags,
 bool DefaultDiagColor, bool DefaultShowOpt) {
@@ -3247,6 +3266,7 @@
   InputArgList Args =
   Opts->ParseArgs(llvm::makeArrayRef(ArgBegin, ArgEnd), MissingArgIndex,
   MissingArgCount, IncludedFlagsBitmask);
+  ParseFEverything(Opts.get(), &Args);
   LangOptions &LangOpts = *Res.getLangOpts();
 
   // Check for missing argument error.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5389,6 +5389,9 @@
 }
   }
 
+  if (Args.hasArg(options::OPT_feverything))
+CmdArgs.push_back("-feverything");
+
   if (Args.hasFlag(options::OPT_faddrsig, options::OPT_fno_addrsig,
(TC.getTriple().isOSBinFormatELF() ||
 TC.getTriple().isOSBinFormatCOFF()) &&
Index: clang/include/clang/Frontend/CompilerInvocation.h
===
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -31,6 +31,8 @@
 namespace opt {
 
 class ArgList;
+class InputArgList;
+class OptTable;
 
 } // namespace opt
 
@@ -61,6 +63,8 @@
  bool DefaultDiagColor = true,
  bool DefaultShowOpt = true);
 
+void ParseFEverything(llvm::opt::OptTable *Opts, llvm::opt::InputArgList 
*Args);
+
 class CompilerInvocationBase {
 public:
   /// Options controlling the language variant.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clan

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Note that with:

1. https://reviews.llvm.org/D60208 ("[X86] Extend boolean arguments to

inline-asm according to getBooleanType")

2. https://reviews.llvm.org/D58260 ("[INLINER] allow inlining of

blockaddresses if sole uses are callbrs")

3. https://reviews.llvm.org/D56571 ("[RFC prototype] Implementation of

asm-goto support in clang")

I can compile a mainline x86 defconfig Linux kernel and boot it in QEMU.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

@compudj email me the preprocessed output of basic_percpu_ops_test.c and I'll 
take a look. (Should be able to find my email via `git log`).


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D56571#1477992 , @jyu2 wrote:

> Please let me know if that is clang problem.


It's an orthogonal issue with Clang's integrated assembler. @compudj confirmed 
that setting `-no-integrated-as` solves the issue (which is what we do 
throughout the kernel, except in a few places mostly by accident).  I'm 
following up with @compudj off thread.

If we can get help w/ code review of:

- https://reviews.llvm.org/D58260
- https://reviews.llvm.org/D60887
- https://reviews.llvm.org/D60224

Then I think we'll have everything we need to land this patch, for the Linux 
kernel's consumption.


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Also: https://reviews.llvm.org/D61560


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

https://reviews.llvm.org/D56571



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: srhines, gbiv.
Herald added a reviewer: george.burgess.iv.
Herald added a subscriber: cfe-commits.
nickdesaulniers removed a reviewer: gbiv.

Fixes PR32985.


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1680,7 +1680,9 @@
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
 if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+&& TypeQuals & Result.getCVRQualifiers() &&
+!S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1680,7 +1680,9 @@
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
 if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+&& TypeQuals & Result.getCVRQualifiers() &&
+!S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166035.
nickdesaulniers added a comment.

git-clang-format HEAD~


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,9 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() && !S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -3,3 +3,8 @@
 int f(int restrict);
 
 void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3; // expected-warning {{extension used}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,9 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() && !S.getLangOpts().GNUMode &&
+DS.getTypeSpecType() != DeclSpec::TST_typeofExpr) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166352.
nickdesaulniers added a comment.

- git-clang-format HEAD~


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,23 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,23 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166355.
nickdesaulniers added a comment.

- move test to new file, use check-prefix for both cases


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166357.
nickdesaulniers added a comment.

- add line numbers to match specific warning lines


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: 7:7: warning: extension used
+// CHECK-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 7:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 12:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 16:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-PEDANTIC: 7:7: warning: extension used
+// CHECK-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 7:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 12:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 16:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic &&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 

lebedev.ri wrote:
> This ideally needs positive tests. E.g.:
> * `-std=c89`
> * `-std=c89 -pedantic`
> * `-std=gnu99`
> * `-std=gnu99 -pedantic`
> * `-std=c99`
> * `-std=c99 -pedantic`
> 
Since `typeof` is a gnu extension, its use constitutes an error for all non gnu 
C standards, so it's moot to check for duplicate const specifiers from typeof 
exprs.

Since we're trying to match GCC's behavior here, GCC does not warn for 
`-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 

nickdesaulniers wrote:
> lebedev.ri wrote:
> > This ideally needs positive tests. E.g.:
> > * `-std=c89`
> > * `-std=c89 -pedantic`
> > * `-std=gnu99`
> > * `-std=gnu99 -pedantic`
> > * `-std=c99`
> > * `-std=c99 -pedantic`
> > 
> Since `typeof` is a gnu extension, its use constitutes an error for all non 
> gnu C standards, so it's moot to check for duplicate const specifiers from 
> typeof exprs.
> 
> Since we're trying to match GCC's behavior here, GCC does not warn for 
> `-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases.
https://godbolt.org/z/3trZdl


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166351.
nickdesaulniers added a comment.

- warn only if -pedantic, add gcc bug test cases


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89.c


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,24 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} 
expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
+
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,14 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode &&
+!S.Diags.getDiagnosticOptions().Pedantic &&
+DS.getTypeSpecType() == DeclSpec::TST_typeofExpr))  {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89.c
===
--- test/Sema/gnu89.c
+++ test/Sema/gnu89.c
@@ -1,5 +1,24 @@
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 
 int f(int restrict);
 
-void main() {} // expected-warning {{return type of 'main' is not 'int'}} expected-note {{change return type to 'int'}}
+void main() {}
+// CHECK: return type of 'main' is not 'int'
+// CHECK: change return type to 'int'
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89.
+const int c_i;
+const typeof(c_i) c_i3;
+// CHECK-PEDANTIC: warning: extension used
+// CHECK-PEDANTIC: warning: duplicate 'const' declaration specifier
+// CHECK-otherwise-NOT: warning: duplicate 'const' declaration specifier
+
+const const int x;
+// CHECK: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t x2;
+// CHECK: warning: duplicate 'const' declaration specifier
+
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,14 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode &&
+!S.Diags.getDiagnosticOptions().Pedantic &&
+DS.getTypeSpecType() == DeclSpec::TST_typeofExpr))  {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166578.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- also run test on gnu99+


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89, unless -pedantic was specified. Do not warn in gnu99+, even
+// with -pedantic.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-GNU89-PEDANTIC: 14:7: warning: extension used
+// CHECK-GNU89-PEDANTIC: 14:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 14:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 19:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 23:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {
+//
+// Not checked for gnu89 if the TST is from a typeof expression and
+// -pedantic was not set.
+if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus &&
+TypeQuals & Result.getCVRQualifiers() &&
+!(S.getLangOpts().GNUMode && !S.Diags.getDiagnosticOptions().Pedantic 
&&
+  DS.getTypeSpecType() == DeclSpec::TST_typeofExpr)) {
   if (TypeQuals & DeclSpec::TQ_const && Result.isConstQualified()) {
 S.Diag(DS.getConstSpecLoc(), diag::ext_duplicate_declspec)
   << "const";


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+// RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-ABSTRUSE %s
+
+// Do not warn about duplicate const declaration specifier as the result of
+// typeof in gnu89, unless -pedantic was specified. Do not warn in gnu99+, even
+// with -pedantic.
+const int c_i;
+const typeof(c_i) c_i2;
+// CHECK-GNU89-PEDANTIC: 14:7: warning: extension used
+// CHECK-GNU89-PEDANTIC: 14:1: warning: duplicate 'const' declaration specifier
+// CHECK-ABSTRUSE-NOT: 14:1: warning: duplicate 'const' declaration specifier
+
+const const int c_i3;
+// CHECK: 19:7: warning: duplicate 'const' declaration specifier
+
+typedef const int t;
+const t c_i4;
+// CHECK: 23:1: warning: duplicate 'const' declaration specifier
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,13 @@
 // C90 6.5.3 constraints: "The same type qualifier shall not appear more
 // than once in the same specifier-list or qualifier-list, either directly
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getC

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaType.cpp:1682
 // or via one or more typedefs."
-if (!S.getLangOpts().C99 && !S.getLangOpts().CPlusPlus
-&& TypeQuals & Result.getCVRQualifiers()) {

srhines wrote:
> This is broken for C11 and C17 (even before you touch anything). As we just 
> talked about, let's have a helper function to detect the oldest version (and 
> maybe even that should get promoted as a LANGOPT).
From the declarations in `include/clang/Frontend/LangStandards.def`, newer 
versions of C bitwise OR together flags of previous versions. So C99 sets C99 
flag, C11 sets C99 and C11 flags, and C17 sets C99, C11, and C17 flags.

So this should be correct (though even to me, this looks wrong on first 
glance).  Thanks to @george.burgess.iv and you for help in looking into this.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/Sema/gnu89.c:1-2
-// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-PEDANTIC %s
+// RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck %s
 

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > lebedev.ri wrote:
> > > This ideally needs positive tests. E.g.:
> > > * `-std=c89`
> > > * `-std=c89 -pedantic`
> > > * `-std=gnu99`
> > > * `-std=gnu99 -pedantic`
> > > * `-std=c99`
> > > * `-std=c99 -pedantic`
> > > 
> > Since `typeof` is a gnu extension, its use constitutes an error for all non 
> > gnu C standards, so it's moot to check for duplicate const specifiers from 
> > typeof exprs.
> > 
> > Since we're trying to match GCC's behavior here, GCC does not warn for 
> > `-std=gnu99` or `-std=gnu99 -pedantic` so I will add those test cases.
> https://godbolt.org/z/3trZdl
Ah, I can still put CHECKs for errors.  Will add additional tests.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52399: [AArch64] Support adding X[8-15, 18] registers as CSRs.

[Nick Desaulniers via Phabricator via cfe-commits]

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

Thanks for this patch, Tri!


Repository:
  rC Clang

https://reviews.llvm.org/D52399



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166737.
nickdesaulniers added a comment.

- remove debug statments


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c

Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,87 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89 %s
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99 %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11 %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17 %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int c_i;
+/*
+CHECK-C89: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i2;
+/*
+CHECK-C89-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 43:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU89-PEDANTIC: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU11-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU11-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU17-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU17-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+*/
+
+const int c_i3;
+const typeof(c_i) c_i4;
+/*
+CHECK-C89: 66:19: error: expected function body after function declarator
+CHECK-C89-PEDANTIC: 66:19: error: expected function body after function declarator
+CHECK-C99: 66:19: error: expected function body after function declarator
+CHECK-C99-PEDANTIC: 66:19: error: expected function body after function declarator
+CHECK-C11: 66:19: error: expected function body after function declarator
+CHECK-C11-PEDANTIC: 66:19: error: expected function body aft

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166736.
nickdesaulniers added a comment.

- add ISO C tests, handle typedef case new tests found


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c

Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,87 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89 %s
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99 %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11 %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17 %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int c_i;
+/*
+CHECK-C89: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i2;
+/*
+CHECK-C89-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 43:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU89-PEDANTIC: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU11-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU11-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU17-NOT: 43:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU17-PEDANTIC-NOT: 43:1: warning: duplicate 'const' declaration specifier
+*/
+
+const int c_i3;
+const typeof(c_i) c_i4;
+/*
+CHECK-C89: 66:19: error: expected function body after function declarator
+CHECK-C89-PEDANTIC: 66:19: error: expected function body after function declarator
+CHECK-C99: 66:19: error: expected function body after function declarator
+CHECK-C99-PEDANTIC: 66:19: error: expected function body after function declarator
+CHECK-C11: 66:19: error: expected function body after function declarator
+CHECK-C11-PEDANTIC: 66:19: err

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166740.
nickdesaulniers added a comment.

- condense CHECK-prefixes into CHECK for const const


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,71 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89 %s
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU99 %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU11 %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU17 %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int c_i;
+/*
+CHECK: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i2;
+/*
+CHECK-C89-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 27:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU89-PEDANTIC: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-CNU11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-CNU17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+*/
+
+const int c_i3;
+const typeof(c_i) c_i4;
+/*
+CHECK-C89: 50:19: error: expected function body after function declarator
+CHECK-C89-PEDANTIC: 50:19: error: expected function body after function 
declarator
+CHECK-C99: 50:19: error: expected function body after function declarator
+CHECK-C99-PEDANTIC: 50:19: error: expected function body after function 
declarator
+CHECK-C11: 50:19: error: expected function body after function declarator
+CHECK-C11-PEDANTIC: 50:19: error: expected function body after function 
declarator
+CHECK-C17: 50:19: error: expected function body after function declarator
+CHECK-C17-PEDANTIC: 50:19: error: expected function body after function 
declarator
+
+CHECK-GNU89-NOT: 50:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU89-PEDANTIC: 50:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 50:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 50:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU11-NOT: 50:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 50:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU17-NOT: 50:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 50:1: warning: duplicate 'const' declaration 
specifier
+*/
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,16 @@

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/Sema/gnu89-const.c:41-46
+CHECK-CNU99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-CNU11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-CNU17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-CNU17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier

gah, `CNU` typo!


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166742.
nickdesaulniers added a comment.

- fix typo s/CNU/GNU/g and update NOTEs


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,71 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89 %s
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU99 %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU11 %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU17 %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int c_i;
+/*
+CHECK: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i2;
+/*
+CHECK-C89-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 27:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 27:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+*/
+
+const int c_i3;
+const typeof(c_i) c_i4;
+/*
+CHECK-C89: 51:19: error: expected function body after function declarator
+CHECK-C89-PEDANTIC: 51:19: error: expected function body after function 
declarator
+CHECK-C99: 51:19: error: expected function body after function declarator
+CHECK-C99-PEDANTIC: 51:19: error: expected function body after function 
declarator
+CHECK-C11: 51:19: error: expected function body after function declarator
+CHECK-C11-PEDANTIC: 51:19: error: expected function body after function 
declarator
+CHECK-C17: 51:19: error: expected function body after function declarator
+CHECK-C17-PEDANTIC: 51:19: error: expected function body after function 
declarator
+
+CHECK-GNU89-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 51:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 51:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU11-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 51:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU17-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 51:1: warning: duplicate 'const' declaration 
specifier
+*/
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,16 @@
 // C90 6

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 166743.
nickdesaulniers added a comment.

- adjust wording in comment


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  lib/Sema/SemaType.cpp
  test/Sema/gnu89-const.c


Index: test/Sema/gnu89-const.c
===
--- /dev/null
+++ test/Sema/gnu89-const.c
@@ -0,0 +1,71 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89 %s
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU99 %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU11 %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU17 %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int c_i;
+/*
+CHECK: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i2;
+/*
+CHECK-C89-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 27:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 27:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU11-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU17-NOT: 27:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 27:1: warning: duplicate 'const' declaration 
specifier
+*/
+
+const int c_i3;
+const typeof(c_i) c_i4;
+/*
+CHECK-C89: 51:19: error: expected function body after function declarator
+CHECK-C89-PEDANTIC: 51:19: error: expected function body after function 
declarator
+CHECK-C99: 51:19: error: expected function body after function declarator
+CHECK-C99-PEDANTIC: 51:19: error: expected function body after function 
declarator
+CHECK-C11: 51:19: error: expected function body after function declarator
+CHECK-C11-PEDANTIC: 51:19: error: expected function body after function 
declarator
+CHECK-C17: 51:19: error: expected function body after function declarator
+CHECK-C17-PEDANTIC: 51:19: error: expected function body after function 
declarator
+
+CHECK-GNU89-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 51:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 51:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU11-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 51:1: warning: duplicate 'const' declaration 
specifier
+CHECK-GNU17-NOT: 51:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 51:1: warning: duplicate 'const' declaration 
specifier
+*/
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -1679,8 +1679,16 @@
 // C90 6.5.3 constra

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

This now matches GCC AFAICT.  My only question is:

Should GCC instead warn for the typedef case for -std=c89 (non pedantic), 
according to C90 6.5.3?


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

> Should GCC instead warn for the typedef case for -std=c89 (non pedantic), 
> according to C90 6.5.3?

Seems like yes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80868#c6
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87435
But let's wait a bit to see what the GCC folks say.  If the newer bug doesn't 
get closed, then I need to rework this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 167565.
nickdesaulniers added a comment.
Herald added a subscriber: jfb.

- split duplicate_declspec into Extension and Extwarn from Extwarn.
- rename ext_duplicate_declspec to be the Extension, not the Extwarn.
- Fix C++ case.
- Use tablegen'd Extension rather than checking Pedantic diag option.
- Update tests that started failing due to this change.
- Consolidate checks for duplicate declspecs into 1 file.
- Move cases for PR8264 into my added test case, extending them for various 
language standards.
- Update my tests/rename the new test file name.


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaType.cpp
  test/FixIt/fixit-eof-space.c
  test/FixIt/fixit.c
  test/Parser/atomic.c
  test/Sema/declspec.c
  test/Sema/dupl-declspec.c

Index: test/Sema/dupl-declspec.c
===
--- /dev/null
+++ test/Sema/dupl-declspec.c
@@ -0,0 +1,198 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int pr8264_1;
+/*
+CHECK-C89-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i;
+/*
+CHECK-C89-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 45:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 45:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 45:1: warning: duplicate 'const' declaration speci

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Ok, I think this is ready for rereview.




Comment at: test/Parser/atomic.c:5
 typedef _Atomic int atomic_int;
-typedef _Atomic _Atomic _Atomic(int) atomic_int; // expected-warning 
{{duplicate '_Atomic' declaration specifier}}
 

Note to reviewers: this deletion was intentional. It is not copied over to the 
new test file test/Sema/dupl-declspec.c because we now DONT want to warn for 
duplicates in C99+, and `_Atomic` was not available before C11.



Comment at: test/Sema/declspec.c:43
-volatile volatile int pr8264_2;  // expected-warning {{duplicate 'volatile' 
declaration specifier}}
-char * restrict restrict pr8264_3;  // expected-warning {{duplicate 'restrict' 
declaration specifier}}
-

Note to reviewers; this deletion was intentional.  It is not copied over to the 
new test file test/Sema/dupl-declspec.c with the rest of the PR8264 cases 
because we now DONT want to warn for duplicates in C99+, and `restrict` was not 
available before then (ie. C90).

The rest of these cases were moved to the new test file 
test/Sema/dupl-declspec.c and exhaustively tested against all current C 
standards.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: test/FixIt/fixit.c:50
-// CHECK: const typedef int int_t;
-const typedef typedef int int_t; // expected-warning {{duplicate 'typedef'}}
-

Ah, this was a case I should add one last test for.  I think reviewers can 
still start reviewing the rest.  This should just add one additional test case.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

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



Comment at: test/FixIt/fixit.c:50
-// CHECK: const typedef int int_t;
-const typedef typedef int int_t; // expected-warning {{duplicate 'typedef'}}
-

nickdesaulniers wrote:
> Ah, this was a case I should add one last test for.  I think reviewers can 
> still start reviewing the rest.  This should just add one additional test 
> case.
Added test case back.



Comment at: test/Parser/atomic.c:5
 typedef _Atomic int atomic_int;
-typedef _Atomic _Atomic _Atomic(int) atomic_int; // expected-warning 
{{duplicate '_Atomic' declaration specifier}}
 

nickdesaulniers wrote:
> Note to reviewers: this deletion was intentional. It is not copied over to 
> the new test file test/Sema/dupl-declspec.c because we now DONT want to warn 
> for duplicates in C99+, and `_Atomic` was not available before C11.
Actually, we can still test that it does not appear.



Comment at: test/Sema/declspec.c:43
-volatile volatile int pr8264_2;  // expected-warning {{duplicate 'volatile' 
declaration specifier}}
-char * restrict restrict pr8264_3;  // expected-warning {{duplicate 'restrict' 
declaration specifier}}
-

nickdesaulniers wrote:
> Note to reviewers; this deletion was intentional.  It is not copied over to 
> the new test file test/Sema/dupl-declspec.c with the rest of the PR8264 cases 
> because we now DONT want to warn for duplicates in C99+, and `restrict` was 
> not available before then (ie. C90).
> 
> The rest of these cases were moved to the new test file 
> test/Sema/dupl-declspec.c and exhaustively tested against all current C 
> standards.
Actually, we can still test that it does not appear.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 167843.
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added a comment.

- add back test for typedef typedef
- add test for dupl _Atomic and restrict


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaType.cpp
  test/FixIt/fixit-eof-space.c
  test/FixIt/fixit.c
  test/Parser/atomic.c
  test/Sema/declspec.c
  test/Sema/dupl-declspec.c

Index: test/Sema/dupl-declspec.c
===
--- /dev/null
+++ test/Sema/dupl-declspec.c
@@ -0,0 +1,242 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: not %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89 -allow-empty %s
+RUN: not %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int pr8264_1;
+/*
+CHECK-C89-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i;
+/*
+CHECK-C89-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 45:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 45:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declar

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 167845.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- move back untouched test case


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaType.cpp
  test/FixIt/fixit-eof-space.c
  test/Parser/atomic.c
  test/Sema/declspec.c
  test/Sema/dupl-declspec.c

Index: test/Sema/dupl-declspec.c
===
--- /dev/null
+++ test/Sema/dupl-declspec.c
@@ -0,0 +1,242 @@
+/*
+RUN: not %clang_cc1 %s -std=c89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89 %s
+RUN: not %clang_cc1 %s -std=c89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C89-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99 %s
+RUN: not %clang_cc1 %s -std=c99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C99-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11 %s
+RUN: not %clang_cc1 %s -std=c11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C11-PEDANTIC %s
+RUN: not %clang_cc1 %s -std=c17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17 %s
+RUN: not %clang_cc1 %s -std=c17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-C17-PEDANTIC %s
+
+RUN: not %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89 -allow-empty %s
+RUN: not %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu99 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu99 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU99-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu11 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu11 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU11-PEDANTIC %s
+RUN: %clang_cc1 %s -std=gnu17 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17 -allow-empty %s
+RUN: %clang_cc1 %s -std=gnu17 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU17-PEDANTIC %s
+*/
+
+const const int pr8264_1;
+/*
+CHECK-C89-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 21:7: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-NOT: 21:7: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 21:7: warning: duplicate 'const' declaration specifier
+*/
+
+typedef const int t;
+const t c_i;
+/*
+CHECK-C89-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C89-PEDANTIC: 45:1: warning: duplicate 'const' declaration specifier
+  ^ NOTE: special case
+CHECK-C99-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C99-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C11-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-C17-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+
+CHECK-GNU89-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 45:1: warning: duplicate 'const' declaration specifier
+^ NOTE: special case
+CHECK-GNU99-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU99-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU11-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-NOT: 45:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU17-PEDANTIC-NOT: 45:1: warning: duplicate 'const' declaration specifier
+*/
+
+const int c_i2;
+const typeof(pr8264_1) c_i

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/DeclSpec.cpp:441-442
   else
-DiagID = IsExtension ? diag::ext_duplicate_declspec :
-   diag::warn_duplicate_declspec;
+DiagID = IsExtension ? diag::ext_duplicate_declspec
+ : diag::extwarn_duplicate_declspec;
   return true;

rsmith wrote:
> This doesn't look like a correct change. When `IsExtension` is false, the 
> duplicate is not ill-formed, so we shouldn't use an `ExtWarn` diagnostic 
> (which results in an error under `-pedantic-errors`).
If the duplicate is not ill-formed, then isn't the original code incorrect, 
since it produces a warning?


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 168183.
nickdesaulniers added a comment.

[SEMA] split ExtWarn dupl-decl-spec's into Extension and ExtWarn

For types deduced from typedef's and typeof's, don't warn for duplicate
declaration specifiers in C90 unless -pedantic.

Create a third diagnostic type for duplicate declaration specifiers.
Previously, we had an ExtWarn and a Warning. This change adds a third,
Extension, which only warns when -pedantic is set, staying silent
otherwise.

Fixes PR32985.


Repository:
  rC Clang

https://reviews.llvm.org/D52248

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  test/Sema/pr32985.c


Index: test/Sema/pr32985.c
===
--- /dev/null
+++ test/Sema/pr32985.c
@@ -0,0 +1,20 @@
+/*
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89 %s -allow-empty
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck 
-check-prefix=CHECK-GNU89-PEDANTIC %s
+*/
+
+typedef const int t;
+const t c_i;
+/*
+CHECK-GNU89-NOT: 7:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+*/
+
+const int c_i2;
+const typeof(c_i2) c_i3;
+/*
+CHECK-GNU89-NOT: 14:7: warning: extension used
+CHECK-GNU89-NOT: 14:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 14:7: warning: extension used
+CHECK-GNU89-PEDANTIC: 14:1: warning: duplicate 'const' declaration specifier
+*/
Index: lib/Sema/DeclSpec.cpp
===
--- lib/Sema/DeclSpec.cpp
+++ lib/Sema/DeclSpec.cpp
@@ -438,7 +438,7 @@
   if (TNew != TPrev)
 DiagID = diag::err_invalid_decl_spec_combination;
   else
-DiagID = IsExtension ? diag::ext_duplicate_declspec :
+DiagID = IsExtension ? diag::ext_warn_duplicate_declspec :
diag::warn_duplicate_declspec;
   return true;
 }
@@ -967,7 +967,7 @@
 unsigned &DiagID) {
   if (isModulePrivateSpecified()) {
 PrevSpec = "__module_private__";
-DiagID = diag::ext_duplicate_declspec;
+DiagID = diag::ext_warn_duplicate_declspec;
 return true;
   }
 
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3838,7 +3838,8 @@
   assert(PrevSpec && "Method did not return previous specifier!");
   assert(DiagID);
 
-  if (DiagID == diag::ext_duplicate_declspec)
+  if (DiagID == diag::ext_duplicate_declspec ||
+  DiagID == diag::ext_warn_duplicate_declspec)
 Diag(Tok, DiagID)
   << PrevSpec << FixItHint::CreateRemoval(Tok.getLocation());
   else if (DiagID == diag::err_opencl_unknown_type_specifier) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -192,7 +192,9 @@
   "flexible array initialization is a GNU extension">, 
InGroup;
 
 // Declarations.
-def ext_duplicate_declspec : ExtWarn<"duplicate '%0' declaration specifier">,
+def ext_duplicate_declspec : Extension<"duplicate '%0' declaration specifier">,
+  InGroup;
+def ext_warn_duplicate_declspec : ExtWarn<"duplicate '%0' declaration 
specifier">,
   InGroup;
 def warn_duplicate_declspec : Warning<"duplicate '%0' declaration specifier">,
   InGroup;


Index: test/Sema/pr32985.c
===
--- /dev/null
+++ test/Sema/pr32985.c
@@ -0,0 +1,20 @@
+/*
+RUN: %clang_cc1 %s -std=gnu89 -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89 %s -allow-empty
+RUN: %clang_cc1 %s -std=gnu89 -pedantic -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-GNU89-PEDANTIC %s
+*/
+
+typedef const int t;
+const t c_i;
+/*
+CHECK-GNU89-NOT: 7:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 7:1: warning: duplicate 'const' declaration specifier
+*/
+
+const int c_i2;
+const typeof(c_i2) c_i3;
+/*
+CHECK-GNU89-NOT: 14:7: warning: extension used
+CHECK-GNU89-NOT: 14:1: warning: duplicate 'const' declaration specifier
+CHECK-GNU89-PEDANTIC: 14:7: warning: extension used
+CHECK-GNU89-PEDANTIC: 14:1: warning: duplicate 'const' declaration specifier
+*/
Index: lib/Sema/DeclSpec.cpp
===
--- lib/Sema/DeclSpec.cpp
+++ lib/Sema/DeclSpec.cpp
@@ -438,7 +438,7 @@
   if (TNew != TPrev)
 DiagID = diag::err_invalid_decl_spec_combination;
   else
-DiagID = IsExtension ? diag::ext_duplicate_declspec :
+DiagID = IsExtension ? diag::ext_warn_duplicate_declspec :
diag::warn_duplicate_declspec;
   return true;
 }
@@ -967,7 +967,7 @@
 unsigned &DiagID) {
   if (isModulePrivat

[PATCH] D52248: [SEMA] ignore duplicate declaration specifiers from typeof exprs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers abandoned this revision.
nickdesaulniers added a comment.

Well that didn't do what I wanted...(sorry, still learning arc).  Going to 
abandon this and push a much simpler fix.


Repository:
  rC Clang

https://reviews.llvm.org/D52248



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