This revision was automatically updated to reflect the committed changes.
Closed by commit rGc310bf8256f8: [Sema] Comparison of pointers to complete and
incomplete types (authored by pestctrl, committed by efriedma).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
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
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.
LGTM
I'll merge for you; how do you want to be credited on the "Author" line of the
commit message?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
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
___
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:
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 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<
pestctrl wrote:
> efriedma wrote:
> > `InGroup`
>
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<
rsmith added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+ Diag(Loc,
+ getLangOpts().C11
+ ? diag::ext_typecheck_compare_complete_incomplete_pointers
efriedma wrote:
> pestctrl wrote:
> > efriedma
efriedma 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<
`InGroup`
Comment at:
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
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:
efriedma added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+ Diag(Loc,
+ getLangOpts().C11
+ ? diag::ext_typecheck_compare_complete_incomplete_pointers
rsmith wrote:
> pestctrl wrote:
> > efriedma
rsmith added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+ Diag(Loc,
+ getLangOpts().C11
+ ? diag::ext_typecheck_compare_complete_incomplete_pointers
pestctrl wrote:
> efriedma wrote:
> > I think this
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 added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:11571
+ Diag(Loc,
+ getLangOpts().C11
+ ? diag::ext_typecheck_compare_complete_incomplete_pointers
I think this condition is backwards? Should be
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
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
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
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:
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:
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:
efriedma 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)">,
pestctrl
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:
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
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
efriedma added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6446
+ "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+ InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
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
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
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:
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
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:
efriedma added a comment.
Any thoughts on "this diagnostic needs to be restricted to C99 or earlier"?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79945/new/
https://reviews.llvm.org/D79945
___
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:
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:
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
efriedma added a comment.
Oh, wait, I was looking at the C11 version. This diagnostic needs to be
restricted to C99 or earlier, and only to relational comparisons, I think.
And given it isn't really a valuable diagnostic, I think it should be off by
default (an "Extension" diagnostic).
efriedma added a comment.
I'm not sure how you derive this requirement from the standard; the section in
question doesn't use the words "complete" or "incomplete" at all. Am I missing
something obvious?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
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
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:
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:
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
42 matches
Mail list logo