[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

@efriedma I think you were the last person to touch this code regarding cases 
where pointer comparisons are invalid. Could you have a look at my changes and 
gimme some feedback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

Ah, you're right. I don't see the clause in the C11 standard. I'll see what I 
can do. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

Hey @efriedma, thanks again for the comments.

Yes, you were correct in that this message should only be emitted for 
relational comparisons.

Regarding having the warning off by default, I did want to point out that GCC 
enables this warning by default: https://c.godbolt.org/z/W_NgYA.

Do you still think that the warning be off by default? I am OK with either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 265807.
pestctrl added a comment.

Changed error to warning, only emit during a relational operation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/compare.c


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of 
complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,21 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   diag::ext_typecheck_comparison_of_complete_and_incomplete_types)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_comparison_of_complete_and_incomplete_types : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,21 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   diag::ext_typecheck_comparison_of_complete_and_incomplete_types)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 265898.
pestctrl added a comment.

Added warning to group c99-extensions, only enable warning when C99 or less


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c89.c


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to 
have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of 
complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,11 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomple

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 265907.
pestctrl added a comment.

Both extension and extwarn need to be in the c99-extensions group


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c89.c


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to 
have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of 
complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incompl

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 265908.
pestctrl added a comment.

Rebase on master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c89.c


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to 
have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of 
complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_ty

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 266086.
pestctrl added a comment.

Rebase on master?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c89.c


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to 
have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of 
complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_ty

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 266214.
pestctrl added a comment.

Forgot to add a comma >_<


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/c89.c


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to 
have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of 
complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/c89.c
===
--- clang/test/Sema/c89.c
+++ clang/test/Sema/c89.c
@@ -127,3 +127,11 @@
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 
+int incomplete[]; /* expected-warning {{tentative array definition assumed to have one element}} */
+int complete[5];
+
+void test18() {
+  if (&incomplete < &complete) { /* expected-warning {{ordered comparison of complete and incomplete pointers}} */
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6447
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,

efriedma wrote:
> warn_typecheck_compare_complete_incomplete_pointers should be a Warning, not 
> an ExtWarn.  And should be DefaultIgnore.  And should indicate why we're 
> warning.  And should be in the group C99Compat.
I'm confused by what you mean for "indicate why we're warning". Should I change 
the warning text?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 266645.
pestctrl marked an inline comment as done.
pestctrl added a comment.

ext_typecheck_compare_complete_incomplete_pointers:

- Moved to group C11.

ext_typecheck_compare_complete_incomplete_pointers:

- Changed to Warning from ExtWarn
- Moved to group C99Compat
- Added DefaultIgnore


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return
+&incomplete <  &complete && // expected-warning {{ordered comparison of 
complete and incomplete pointers}}
+&incomplete <= &complete && // expected-warning {{ordered comparison of 
complete and incomplete pointers}}
+&incomplete >  &complete && // expected-warning {{ordered comparison of 
complete and incomplete pointers}}
+&incomplete >= &complete && // expected-warning {{ordered comparison of 
complete and incomplete pointers}}
+&incomplete == &complete &&
+&incomplete != &complete;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return
+&incomplete <  &

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 266880.
pestctrl added a comment.

clang-format for the test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete <= &complete) && // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete > &complete) &&  // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete >= &complete) && // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11422,11 +11422,23 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6441,6 +6441,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{ordered comparison of complete and incomplete pointers}}
+ (&incomplete <= &complete) && // expected-warning {{ordered comparison of complete and incompl

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267876.
pestctrl added a comment.

Updated warning message to be more descriptive


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete <= &complete) && // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete > &complete) &&  // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete >= &complete) && // expected-warning {{ordered 
comparison of complete and incomplete pointers}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int 

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267901.
pestctrl added a comment.

Updated test with new error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+i

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267920.
pestctrl added a comment.

Tests need to contain the full error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-r

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267947.
pestctrl added a comment.

Copy pasted error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-rela

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 267950.
pestctrl added a comment.

Copy pasted error messages


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

@efriedma Any more comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers

efriedma wrote:
> I think this condition is backwards?  Should be `!getLangOpts().C11`.  You 
> want the warning with `-std=c99 -pedantic`, you don't want the warning with 
> `std=c11 -pedantic`.
I don't think it's backwards. If getLangOpts().C11, then it is an extension. 
Otherwise, it is the warning. I can switch the conditions if it is confusing 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 269824.
pestctrl added a comment.

Don't diagnose outside of C99


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,22 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if ((LCanPointeeTy->isIncompleteType() !=
+ RCanPointeeTy->isIncompleteType()) &&
+!getLangOpts().C11) {
+  Diag(Loc, diag::ext_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer c

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers

efriedma wrote:
> rsmith wrote:
> > pestctrl wrote:
> > > efriedma wrote:
> > > > I think this condition is backwards?  Should be `!getLangOpts().C11`.  
> > > > You want the warning with `-std=c99 -pedantic`, you don't want the 
> > > > warning with `std=c11 -pedantic`.
> > > I don't think it's backwards. If getLangOpts().C11, then it is an 
> > > extension. Otherwise, it is the warning. I can switch the conditions if 
> > > it is confusing though.
> > "Extension" means "this is invalid code that we're accepting anyway" -- 
> > that's what this is in C99. In C11, I think we shouldn't be diagnosing at 
> > all.
> > 
> > Has anyone checked whether WG14 removed this restriction in C11 as a DR 
> > resolution? If so, we shouldn't be diagnosing it at all, in any language 
> > mode.
> I tracked down the proposal for the change; it's 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1439.pdf .  Beyond the 
> reference to http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_314.htm , I 
> can't find any relevant defect report.
I have updated the diff to diagnose only for C99. Does the existence of the 
proposal mean we shouldn't be diagnosing in any language mode?

Also, how did you track down the proposal that quickly? Even after skimming 
through it, I still can't find it through searching. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6451
+  "%1 is %select{|in}3complete">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<

efriedma wrote:
> `InGroup`
Sorry, I'm not sure I understand. Isn't this a C99 warning? Why is it being put 
in the C11 group?



Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers

rsmith wrote:
> efriedma wrote:
> > pestctrl wrote:
> > > efriedma wrote:
> > > > rsmith wrote:
> > > > > pestctrl wrote:
> > > > > > efriedma wrote:
> > > > > > > I think this condition is backwards?  Should be 
> > > > > > > `!getLangOpts().C11`.  You want the warning with `-std=c99 
> > > > > > > -pedantic`, you don't want the warning with `std=c11 -pedantic`.
> > > > > > I don't think it's backwards. If getLangOpts().C11, then it is an 
> > > > > > extension. Otherwise, it is the warning. I can switch the 
> > > > > > conditions if it is confusing though.
> > > > > "Extension" means "this is invalid code that we're accepting anyway" 
> > > > > -- that's what this is in C99. In C11, I think we shouldn't be 
> > > > > diagnosing at all.
> > > > > 
> > > > > Has anyone checked whether WG14 removed this restriction in C11 as a 
> > > > > DR resolution? If so, we shouldn't be diagnosing it at all, in any 
> > > > > language mode.
> > > > I tracked down the proposal for the change; it's 
> > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1439.pdf .  Beyond the 
> > > > reference to http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_314.htm 
> > > > , I can't find any relevant defect report.
> > > I have updated the diff to diagnose only for C99. Does the existence of 
> > > the proposal mean we shouldn't be diagnosing in any language mode?
> > > 
> > > Also, how did you track down the proposal that quickly? Even after 
> > > skimming through it, I still can't find it through searching. 
> > > Also, how did you track down the proposal that quickly? 
> > 
> > It wasn't really that quick, but the thing that eventually worked was that 
> > I googled for `site:open-std.org "At various points within a translation 
> > unit"`.
> I tried to get some clarity from WG14 as to whether they intended N1439 to be 
> interpreted as applying retroactively, but it seems like their stance is that 
> they do not do maintenance work on past standards, and have no mechanism for 
> identifying whether papers should be encouraged for retroactive application 
> or only for implementations intending to conform to later standards.
> 
> In the absence of guidance either way from WG14, I think our best bet is to 
> follow GCC and the literal standards text as this patch does.
GCC has left this warning on by default in any language mode, FWIW. Should we 
still restrict this warning to only C99 mode? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6451
+  "%1 is %select{|in}3complete">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<

rsmith wrote:
> pestctrl wrote:
> > efriedma wrote:
> > > `InGroup`
> > Sorry, I'm not sure I understand. Isn't this a C99 warning? Why is it being 
> > put in the C11 group?
> Because `C11` really means `C11Extensions`, and this is a C11 extension (ie, 
> it's code that's valid in C11 but not valid in C99):
> ```
> // A warning group for warnings about using C11 features as extensions.
> def C11 : DiagGroup<"c11-extensions">;
> ```
Got it, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 270139.
pestctrl added a comment.

Moved the extension to C11 group


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc11-extensions %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete <= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete > &complete) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete >= &complete) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ (&incomplete == &complete) &&
+ (&incomplete != &complete);
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,22 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if ((LCanPointeeTy->isIncompleteType() !=
+ RCanPointeeTy->isIncompleteType()) &&
+!getLangOpts().C11) {
+  Diag(Loc, diag::ext_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,12 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc11-extensions %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return (&incomplete < &complete) &&  // expected-

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl marked 5 inline comments as done.
pestctrl added a comment.

@rsmith @efriedma Any more comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

Benson Chu 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl created this revision.
pestctrl added a reviewer: rsmith.
pestctrl added a project: clang.

Clang is missing one of the conditions for C99 6.5.9p2, where comparison 
between pointers must either both point to incomplete types or both point to 
complete types. This patch adds an extra check to the clause where two pointers 
are of compatible types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/compare.c


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[];
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of 
complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11426,8 +11426,15 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
+  // Pointers both need to point to complete or incomplete types
+  if (LCanPointeeTy->isIncompleteType() !=
+  RCanPointeeTy->isIncompleteType()) {
+Diag(Loc,
+ diag::err_typecheck_comparison_of_complete_and_incomplete_types)
+<< LHSType << RHSType << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+  } else if (IsRelational && LCanPointeeTy->isFunctionType()) {
+// Valid unless a relational comparison of function pointers
 Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
   << LHSType << RHSType << LHS.get()->getSourceRange()
   << RHS.get()->getSourceRange();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6461,6 +6461,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def err_typecheck_comparison_of_complete_and_incomplete_types : Error<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[];
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11426,8 +11426,15 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
+  // Pointers both need to point to complete or incomplete types
+  if (LCanPointeeTy->isIncompleteType() !=
+  RCanPointeeTy->isIncompleteType()) {
+Diag(Loc,
+ diag::err_typecheck_comparison_of_complete_and_incomplete_types)
+<< LHSType << RHSType << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+  } else if (IsRelational && LCanPointeeTy->isFunctionType()) {
+// Valid unless a relational comparison of function pointers
 Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
   << LHSType << RHSType << LHS.get()->getSourceRange()
   << RHS.get()->getSourceRange();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6461,6 +6461,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison betwe

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 264089.
pestctrl added a comment.

Updated test to also expect a warning along with the newly added error.


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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/compare.c


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of 
complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11426,8 +11426,15 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
+  // Pointers both need to point to complete or incomplete types
+  if (LCanPointeeTy->isIncompleteType() !=
+  RCanPointeeTy->isIncompleteType()) {
+Diag(Loc,
+ diag::err_typecheck_comparison_of_complete_and_incomplete_types)
+<< LHSType << RHSType << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+  } else if (IsRelational && LCanPointeeTy->isFunctionType()) {
+// Valid unless a relational comparison of function pointers
 Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
   << LHSType << RHSType << LHS.get()->getSourceRange()
   << RHS.get()->getSourceRange();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6461,6 +6461,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def err_typecheck_comparison_of_complete_and_incomplete_types : Error<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11426,8 +11426,15 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
+  // Pointers both need to point to complete or incomplete types
+  if (LCanPointeeTy->isIncompleteType() !=
+  RCanPointeeTy->isIncompleteType()) {
+Diag(Loc,
+ diag::err_typecheck_comparison_of_complete_and_incomplete_types)
+<< LHSType << RHSType << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+  } else if (IsRelational && LCanPointeeTy->isFunctionType()) {
+// Valid unless a relational comparison of function pointers
 Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
   << LHSType << RHSType << LHS.get()->getSourceRange()
   << RHS.get()->getSourceRange();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6461,6 +6461,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 264697.
pestctrl edited the summary of this revision.
pestctrl added a comment.

Rebased on top of master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/compare.c


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of 
complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11424,8 +11424,15 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
+  // Pointers both need to point to complete or incomplete types
+  if (LCanPointeeTy->isIncompleteType() !=
+  RCanPointeeTy->isIncompleteType()) {
+Diag(Loc,
+ diag::err_typecheck_comparison_of_complete_and_incomplete_types)
+<< LHSType << RHSType << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+  } else if (IsRelational && LCanPointeeTy->isFunctionType()) {
+// Valid unless a relational comparison of function pointers
 Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
   << LHSType << RHSType << LHS.get()->getSourceRange()
   << RHS.get()->getSourceRange();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6434,6 +6434,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def err_typecheck_comparison_of_complete_and_incomplete_types : Error<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -405,3 +405,12 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to have one element}}
+int complete[5];
+
+void test13() {
+  if (&incomplete < &complete) { // expected-error {{ordered comparison of complete and incomplete pointers}}
+return;
+  }
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11424,8 +11424,15 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
+  // Pointers both need to point to complete or incomplete types
+  if (LCanPointeeTy->isIncompleteType() !=
+  RCanPointeeTy->isIncompleteType()) {
+Diag(Loc,
+ diag::err_typecheck_comparison_of_complete_and_incomplete_types)
+<< LHSType << RHSType << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+  } else if (IsRelational && LCanPointeeTy->isFunctionType()) {
+// Valid unless a relational comparison of function pointers
 Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
   << LHSType << RHSType << LHS.get()->getSourceRange()
   << RHS.get()->getSourceRange();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6434,6 +6434,8 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way compa

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 366513.
pestctrl added a comment.

Update comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section 
"datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2216,14 +2216,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first (most recent) TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2216,14 +2216,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first (most recent) TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 366522.
pestctrl added a comment.

Only look for attributes in check string for unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr ={{.*}}section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2216,14 +2216,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first (most recent) TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr ={{.*}}section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2216,14 +2216,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first (most recent) TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

@mizvekov Thanks for the help! I recently got commit access, so I think I can 
commit this myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

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

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

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

@rsmith Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

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

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl created this revision.
pestctrl added reviewers: akyrtzi, rsmith.
pestctrl added a project: clang.
pestctrl requested review of this revision.
Herald added a subscriber: cfe-commits.

I noticed this bug because attributes were being dropped from tentative
definitions after the second tentative definition. If you have the following C
code:

char chararr[13];
char chararr[13] __attribute__ ((section("data")));
char chararr[13] __attribute__ ((aligned(16)));

If you emit the LLVM IR, the chararr will have the section attribute, but not
the aligned attribute. If you have more tentative definitions following the
last line, those attributes will be dropped as well.

This is not a problem with merging of attributes, as that works completely
fine. Clang will store a merged list of attributes in the most recent tentative
definition encountered. The problem is that clang will choose one of the
tentative definitions to emit into LLVM IR, and it's NOT the most recent
tentative definition.

VarDecl::getActingDefinition is called to choose the which tentative definition
should be emitted. VarDecl::getActingDefinition loops through VarDecl::redecls,
which will return an iterator of all redeclarations of a variable, which will
include all tentative definitions. VarDecl::getActingDefinition assumes that
the order of this iterator is the order in which the defs/decls appear in the
file, but this is not the case. The first element of the iterator is in fact
the first def/decl, but the rest of the elements are reversed. So, if we had 4
def/decls, the order they appear in the iterator would be [1, 4, 3, 2].

So, when VarDecl::getActingDefinition loops through VarDecl::redecls, picking
the last element in the iterator, it's actually picking the second
decl/def. Because merged attributes appear in the most recent def/decl, any
attributes that appear after the second decl are dropped.

This changeset modifies VarDecl::getActingDefinition to use some helper
functions - namely getMostRecentDecl and getPreviousDecl - to instead loop
through the declaration chain in reverse order of how they appear in the file,
returning the first instance that is a tentative definition. This means
attributes will no longer be dropped.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section 
"datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2189,19 +2189,20 @@
 
 VarDecl *VarDecl::getActingDefinition() {
   DefinitionKind Kind = isThisDeclarationADefinition();
-  if (Kind != TentativeDefinition)
+  if (Kind != TentativeDefinition || hasDefinition())
 return nullptr;
 
-  VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
-if (Kind == Definition)
-  return nullptr;
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
+
+// Return the first TentativeDefinition that is encountered.
 if (Kind == TentativeDefinition)
-  LastTentative = I;
+  return Decl;
   }
-  return LastTentative;
+
+  return nullptr;
 }
 
 VarDecl *VarDecl::getDefinition(ASTContext &C) {


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2189,19 +2189,20 @@
 
 VarDecl *VarDecl::getActingDefinition() {
   DefinitionKind Kind = isThisDeclarationADefinition();
-  if (Kind != TentativeDefinition)
+  if (Kind != TentativeDefinition || hasDefinition())
 return nullptr;
 
-  VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
-if (Kind == Definition)
-  return nullptr;
+  

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl updated this revision to Diff 335113.
pestctrl edited the summary of this revision.
pestctrl added a comment.

Removed extra pass over decl chain for acting definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section 
"datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2193,14 +2193,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr = dso_local global [10 x i8] zeroinitializer, section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2193,14 +2193,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl added inline comments.



Comment at: clang/lib/AST/Decl.cpp:2192
   DefinitionKind Kind = isThisDeclarationADefinition();
-  if (Kind != TentativeDefinition)
+  if (Kind != TentativeDefinition || hasDefinition())
 return nullptr;

rsmith wrote:
> Is there a good reason to switch from checking for a `Definition` in the loop 
> to checking it ahead of time? This change means we'll do two passes over the 
> redeclarations, and call `isThisDeclarationADefinition` twice for each, where 
> the old approach only performed one pass.
My thought was that separating the logic would make the loop easier to read, 
but I see that this causes 2 passes vs. 1. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

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

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

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

[PATCH] D99732: [AST] Pick last tentative definition as the acting definition

[Benson Chu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a5f38885056: [AST] Pick last tentative definition as the 
acting definition (authored by pestctrl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99732

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/attr-tentative-definition.c


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr ={{.*}}section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2216,14 +2216,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first (most recent) TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 


Index: clang/test/CodeGen/attr-tentative-definition.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-tentative-definition.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+char arr[10];
+char arr[10] __attribute__((section("datadata")));
+char arr[10] __attribute__((aligned(16)));
+
+// CHECK: @arr ={{.*}}section "datadata", align 16
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2216,14 +2216,18 @@
 return nullptr;
 
   VarDecl *LastTentative = nullptr;
-  VarDecl *First = getFirstDecl();
-  for (auto I : First->redecls()) {
-Kind = I->isThisDeclarationADefinition();
+
+  // Loop through the declaration chain, starting with the most recent.
+  for (VarDecl *Decl = getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl()) {
+Kind = Decl->isThisDeclarationADefinition();
 if (Kind == Definition)
   return nullptr;
-if (Kind == TentativeDefinition)
-  LastTentative = I;
+// Record the first (most recent) TentativeDefinition that is encountered.
+if (Kind == TentativeDefinition && !LastTentative)
+  LastTentative = Decl;
   }
+
   return LastTentative;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits