This revision was automatically updated to reflect the committed changes.
Closed by commit rL357577: Fix typos in tests. NFC. (authored by Higuoxing,
committed by ).
Herald added a subscriber: delcypher.
Changed prior to commit:
https://reviews.llvm.org/D60183?vs=193439&id=193482#toc
Repositor
Higuoxing created this revision.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, kubamracek.
Herald added projects: clang, Sanitizers, LLVM.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D60183
Files:
clang/test/Analysis/analyzer-list-configs.c
compiler-rt/t
Higuoxing added inline comments.
Comment at: clang/include/clang/Format/Format.h:247
+ /// When used in conjuction with ``AllowShortIfIfStatementsOnASingleLine``
+ /// then when ``true``, ``if (a) return;`` can be put on a single even when
Small nit:
```
Al
Higuoxing reclaimed this revision.
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1288272, @vsapsai wrote:
> Sorry, you've decided to abandon the patch, it took a lot of good work. Xing,
> are you sure you don't want to see this change finished?
No, I am working on this :)
> I
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1266893, @vsapsai wrote:
> Sorry about the delay. The change seems to be correct but `ninja check-clang`
> reveals the test "Misc/caret-diags-macros.c" is failing. Can you please look
> into that?
Hi, It's because the expression is
Higuoxing added a comment.
Ping. Now, this patch could give fix-it note on parentheses in macros.
https://reviews.llvm.org/D47687
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Higuoxing updated this revision to Diff 166583.
Higuoxing added a comment.
I update the *SuggestParentheses* function, now, it could deal with parentheses
inside macros
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1154707, @vsapsai wrote:
> Hope this gives you an idea how you can try to make fix-its work.
Hi, Thank you @vsapsai , Yes, I am afraid if I add some extra function and may
cause some problems. Because, this API `getLocForEndOfToken`
Higuoxing added a comment.
Sorry, It seems a little bit difficult for me to add a proper fix-it hint for
expressions in macros, because I cannot find the exact position of the last
char of the token on right hand side of operator. Any suggestion?
Actually, in gcc, it will emit warning for every
Higuoxing updated this revision to Diff 151736.
Higuoxing added a comment.
test case has been passed
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
---
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1135284, @dexonsmith wrote:
> Did you miss this comment from my previous review?
>
> > I would like to see tests like the following:
> >
> > NESTING_VOID_WRAPPER(&&, ||, i, i, i && i); // no warning.
> > NESTING_VOID_WRAPPER(||, &
Higuoxing updated this revision to Diff 151661.
Higuoxing marked 12 inline comments as done.
Higuoxing removed a reviewer: echristo.
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
==
Higuoxing added inline comments.
Comment at: test/Sema/parentheses.c:114
+ NON_NESTING_VOID_0(i && i || i); // expected-warning {{'&&' within '||'}} \
+ // expected-note {{place parentheses around
the '&&' expression to silence this warning}}
+
Higuoxing updated this revision to Diff 151654.
Higuoxing marked an inline comment as done.
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
--- test/Sema/p
Higuoxing marked 9 inline comments as done.
Higuoxing added inline comments.
Comment at: test/Sema/parentheses.c:20
+#define NESTING_VOID(cond) ( (void)(cond) )
+#define NESTING_VOID_WRAPPER(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )
+
dexonsmith wrote:
> - You
Higuoxing added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:12304-12309
// Diagnose "arg1 & arg2 | arg3"
if ((Opc == BO_Or || Opc == BO_Xor) &&
!OpLoc.isMacroID()/* Don't warn in macros. */) {
DiagnoseBitwiseOpInBitwiseOp(Self, Opc, OpLoc, LHSExpr);
Higuoxing updated this revision to Diff 151633.
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -
Higuoxing updated this revision to Diff 151605.
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@ -
Higuoxing updated this revision to Diff 151604.
Higuoxing added a comment.
Ping, thanks
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
--- test/Sema/pare
Higuoxing updated this revision to Diff 151599.
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
===
--- test/Sema/parentheses.c
+++ test/Sema/parentheses.c
@@
Higuoxing added a comment.
Ping
https://reviews.llvm.org/D47687
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Higuoxing updated this revision to Diff 151460.
Higuoxing added a comment.
I think I am almost there.
Here, In my sight
#define foo(op0, op1, x, y, z) ( (void)(x op0 y op1 z) )
is un-actionable, because `x op0 y op1 z` are from different arguments of
function-like macro, so, we will not chec
Higuoxing updated this revision to Diff 151290.
Higuoxing added a comment.
Ping, well, I think I understand `@dexonsmith`'s idea.
`Actionable` macro argument is just like something like this
#define foo(x) ( (void)(x) )
when we using it by
foo(a && b || c);
we could add parentheses for it
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1128757, @dexonsmith wrote:
>
> Yes, I think understand the patch; but I think it's the wrong direction. I
> think we should just make the existing `-Wlogical-op-parentheses` smart
> enough to show actionable warnings in macros
Higuoxing marked 3 inline comments as done.
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1126607, @dexonsmith wrote:
> In https://reviews.llvm.org/D47687#1126074, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D47687#1126032, @Higuoxing wrote:
> >
> > > In https://reviews.l
Higuoxing added a comment.
This diff version pass the both `parentheses.c` and `logical-op-parentheses.c`
https://reviews.llvm.org/D47687
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
Higuoxing updated this revision to Diff 150517.
https://reviews.llvm.org/D47687
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/Sema/logical-op-parentheses-in-macros.c
Index: test/Sema/logical-op-parentheses-in-macros.c
Higuoxing marked 2 inline comments as done.
Higuoxing added a comment.
Sorry, I will check it and update the test case
Besides, shall I add the macro-parentheses checking test cases to the original
'parentheses.c'?
https://reviews.llvm.org/D47687
Higuoxing updated this revision to Diff 150507.
Higuoxing marked 4 inline comments as done.
Higuoxing added a comment.
Hope this not disturb you too much : ) thanks
https://reviews.llvm.org/D47687
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
li
Higuoxing updated this revision to Diff 150495.
Higuoxing added a comment.
Update without conflict with test case `parentheses.c`... I add a separate
option [-Wlogical-op-parentheses-in-macros] and will be silence by default
thanks
https://reviews.llvm.org/D47687
Files:
include/clang/Basic/
Higuoxing added a comment.
Thanks, let me have a try
https://reviews.llvm.org/D47687
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Higuoxing updated this revision to Diff 150465.
Higuoxing added a comment.
I step out a little further... I made an attempt to add a new warning
`[-Wlogical-op-parentheses-in-macros]`. Comparing to the previous one, which do
you prefer? I am new to llvm community, and as you see, this is my firs
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1126074, @lebedev.ri wrote:
> At worst, we can issue this warning in a new `-Wparentheses-in-macros`
> subgroup, which, if apple so insists, could be off-by-default.
> That would less worse than just completely silencing it for the e
Higuoxing added a comment.
In https://reviews.llvm.org/D47687#1125926, @rnk wrote:
> @dexonsmith is there someone from Apple who can comment on rdar://8678458 and
> the merits of disabling this warning in macros? I strongly suspect the
> original report was dealing with code like `assert(x || y
Higuoxing updated this revision to Diff 150253.
Higuoxing added a reviewer: echristo.
Higuoxing added a comment.
update with comments & remove some useless blank lines.
https://reviews.llvm.org/D47687
Files:
lib/Sema/SemaExpr.cpp
test/Sema/parentheses.c
Index: test/Sema/parentheses.c
35 matches
Mail list logo
