vsapsai added a comment.
OK. Good to know you are still working on it.
In https://reviews.llvm.org/D47687#1271880, @Higuoxing wrote:
> 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
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
vsapsai added a comment.
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? I agree that delays in
code review can be frustrating and I think it is something we can improve.
https://reviews.llvm.org/D47687
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
vsapsai added a comment.
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?
Appreciate your contribution, Xing, and thanks for verifying your change with
tests.
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
vsapsai added a comment.
In https://reviews.llvm.org/D47687#1136351, @Higuoxing wrote:
> 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
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
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(||,
dexonsmith added a comment.
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(||, &&, i, i, i || i); // no warning.
Comment at:
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
===
---
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:
> -
dexonsmith added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:12236
+/// Look for '&&' in the righ or left hand of a '||' expr
+static void DiagnoseLogicalAndInLogicalOr(Sema , SourceLocation OpLoc,
- Please add a period at the end of the sentence.
-
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 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
===
---
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
@@
dexonsmith added reviewers: arphaman, ahatanak.
dexonsmith added subscribers: arphaman, ahatanak.
dexonsmith added a comment.
In https://reviews.llvm.org/D47687#1133222, @Higuoxing wrote:
> I think I am almost there.
>
> Here, In my sight
>
> #define foo(op0, op1, x, y, z) ( (void)(x op0 y op1
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
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
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
dexonsmith added a comment.
In https://reviews.llvm.org/D47687#1127120, @Higuoxing wrote:
> 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:
28 matches
Mail list logo
