Re: Patch submission for bug 27400
Thank you for the patch! Note, however, that most clang-tidy reviews are
done using Phabricator (see llvm.org/docs/Phabricator.html). It's not
required, but it makes the reviews much easier (and much easier to keep
track of).
On Tue, May 17, 2016 at 10:47 PM, Mads Ravn via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Cool :) don't the sweat the time. I was just a little excited. Small patch
> but it's nice to get started somewhere.
>
> Best regards,
> Mads Ravn
>
> > On May 17, 2016, at 2:59 AM, Mads Ravn wrote:
> >
> > Hi guys,
> >
> > I just wanted to check up on this patch. I heard I could just reply to
> this mail and see if I could 'ping' anyone in this regard. Hope it's OK.
>
> Sorry for the delay! This looks good. Committed as r269786.
>
> thanks,
> vedant
>
> >
> > Best regards,
> > Mads Ravn
> >
> > On Thu, May 12, 2016 at 6:11 PM Mads Ravn wrote:
> > Hi,
> >
> > I have fixed the things you mentioned now. I have attached the new patch
> to this email.
> >
> > Best regards,
> > Mads Ravn
> >
> > On Wed, May 11, 2016 at 11:54 PM Vedant Kumar wrote:
> > Hi,
> >
> > Thanks for the patch!
> >
> > This patch is missing a small, lit-style test case. You can find
> examples of test cases here:
> >
> > extra/test/clang-tidy/
> >
> > Apart from that, my only other nit-pick is that llvm uses 2-space
> indents, and spaces between "if" and "(".
> >
> > If you reply to this list with an updated patch, someone would be happy
> to commit it for you.
> >
> > best
> > vedant
> >
> > > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> > >
> > > Hi,
> > >
> > > I would like to submit a patch for
> https://llvm.org/bugs/show_bug.cgi?id=27400 .
> > >
> > > Beside attaching the patch, is there anything I should be aware of? I
> have not submitted a patch before.
> > >
> > > You can find the patch attached to this mail.
> > >
> > > Kind regards,
> > > Mads Ravn
> > >
> ___
> > > cfe-commits mailing list
> > > cfe-commits@lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch submission for bug 27400
Cool :) don't the sweat the time. I was just a little excited. Small patch
but it's nice to get started somewhere.
Best regards,
Mads Ravn
> On May 17, 2016, at 2:59 AM, Mads Ravn wrote:
>
> Hi guys,
>
> I just wanted to check up on this patch. I heard I could just reply to
this mail and see if I could 'ping' anyone in this regard. Hope it's OK.
Sorry for the delay! This looks good. Committed as r269786.
thanks,
vedant
>
> Best regards,
> Mads Ravn
>
> On Thu, May 12, 2016 at 6:11 PM Mads Ravn wrote:
> Hi,
>
> I have fixed the things you mentioned now. I have attached the new patch
to this email.
>
> Best regards,
> Mads Ravn
>
> On Wed, May 11, 2016 at 11:54 PM Vedant Kumar wrote:
> Hi,
>
> Thanks for the patch!
>
> This patch is missing a small, lit-style test case. You can find examples
of test cases here:
>
> extra/test/clang-tidy/
>
> Apart from that, my only other nit-pick is that llvm uses 2-space
indents, and spaces between "if" and "(".
>
> If you reply to this list with an updated patch, someone would be happy
to commit it for you.
>
> best
> vedant
>
> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> >
> > Hi,
> >
> > I would like to submit a patch for
https://llvm.org/bugs/show_bug.cgi?id=27400 .
> >
> > Beside attaching the patch, is there anything I should be aware of? I
have not submitted a patch before.
> >
> > You can find the patch attached to this mail.
> >
> > Kind regards,
> > Mads Ravn
> >
___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch submission for bug 27400
> On May 17, 2016, at 2:59 AM, Mads Ravn wrote:
>
> Hi guys,
>
> I just wanted to check up on this patch. I heard I could just reply to this
> mail and see if I could 'ping' anyone in this regard. Hope it's OK.
Sorry for the delay! This looks good. Committed as r269786.
thanks,
vedant
>
> Best regards,
> Mads Ravn
>
> On Thu, May 12, 2016 at 6:11 PM Mads Ravn wrote:
> Hi,
>
> I have fixed the things you mentioned now. I have attached the new patch to
> this email.
>
> Best regards,
> Mads Ravn
>
> On Wed, May 11, 2016 at 11:54 PM Vedant Kumar wrote:
> Hi,
>
> Thanks for the patch!
>
> This patch is missing a small, lit-style test case. You can find examples of
> test cases here:
>
> extra/test/clang-tidy/
>
> Apart from that, my only other nit-pick is that llvm uses 2-space indents,
> and spaces between "if" and "(".
>
> If you reply to this list with an updated patch, someone would be happy to
> commit it for you.
>
> best
> vedant
>
> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits
> > wrote:
> >
> > Hi,
> >
> > I would like to submit a patch for
> > https://llvm.org/bugs/show_bug.cgi?id=27400 .
> >
> > Beside attaching the patch, is there anything I should be aware of? I have
> > not submitted a patch before.
> >
> > You can find the patch attached to this mail.
> >
> > Kind regards,
> > Mads Ravn
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch submission for bug 27400
Hi guys,
I just wanted to check up on this patch. I heard I could just reply to this
mail and see if I could 'ping' anyone in this regard. Hope it's OK.
Best regards,
Mads Ravn
On Thu, May 12, 2016 at 6:11 PM Mads Ravn wrote:
> Hi,
>
> I have fixed the things you mentioned now. I have attached the new patch
> to this email.
>
> Best regards,
> Mads Ravn
>
> On Wed, May 11, 2016 at 11:54 PM Vedant Kumar wrote:
>
>> Hi,
>>
>> Thanks for the patch!
>>
>> This patch is missing a small, lit-style test case. You can find examples
>> of test cases here:
>>
>> extra/test/clang-tidy/
>>
>> Apart from that, my only other nit-pick is that llvm uses 2-space
>> indents, and spaces between "if" and "(".
>>
>> If you reply to this list with an updated patch, someone would be happy
>> to commit it for you.
>>
>> best
>> vedant
>>
>> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Hi,
>> >
>> > I would like to submit a patch for
>> https://llvm.org/bugs/show_bug.cgi?id=27400 .
>> >
>> > Beside attaching the patch, is there anything I should be aware of? I
>> have not submitted a patch before.
>> >
>> > You can find the patch attached to this mail.
>> >
>> > Kind regards,
>> > Mads Ravn
>> >
>> ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch submission for bug 27400
Hi,
I have fixed the things you mentioned now. I have attached the new patch to
this email.
Best regards,
Mads Ravn
On Wed, May 11, 2016 at 11:54 PM Vedant Kumar wrote:
> Hi,
>
> Thanks for the patch!
>
> This patch is missing a small, lit-style test case. You can find examples
> of test cases here:
>
> extra/test/clang-tidy/
>
> Apart from that, my only other nit-pick is that llvm uses 2-space indents,
> and spaces between "if" and "(".
>
> If you reply to this list with an updated patch, someone would be happy to
> commit it for you.
>
> best
> vedant
>
> > On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Hi,
> >
> > I would like to submit a patch for
> https://llvm.org/bugs/show_bug.cgi?id=27400 .
> >
> > Beside attaching the patch, is there anything I should be aware of? I
> have not submitted a patch before.
> >
> > You can find the patch attached to this mail.
> >
> > Kind regards,
> > Mads Ravn
> >
> ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
Index: clang-tidy/misc/MacroParenthesesCheck.cpp
===
--- clang-tidy/misc/MacroParenthesesCheck.cpp (revision 268956)
+++ clang-tidy/misc/MacroParenthesesCheck.cpp (working copy)
@@ -184,6 +184,9 @@
Next.isOneOf(tok::comma, tok::greater))
continue;
+if (Prev.is(tok::kw_namespace))
+ continue;
+
Check->diag(Tok.getLocation(), "macro argument should be enclosed in "
"parentheses")
<< FixItHint::CreateInsertion(Tok.getLocation(), "(")
Index: test/clang-tidy/misc-macro-parentheses.cpp
===
--- test/clang-tidy/misc-macro-parentheses.cpp (revision 268956)
+++ test/clang-tidy/misc-macro-parentheses.cpp (working copy)
@@ -36,6 +36,7 @@
#define GOOD25(t) std::set s
#define GOOD26(x) (a->*x)
#define GOOD27(x) (a.*x)
+#define GOOD28(x) namespace x {int b;}
// These are allowed for now..
#define MAYBE1*12.34
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: Patch submission for bug 27400
Hi,
Thanks for the patch!
This patch is missing a small, lit-style test case. You can find examples of
test cases here:
extra/test/clang-tidy/
Apart from that, my only other nit-pick is that llvm uses 2-space indents, and
spaces between "if" and "(".
If you reply to this list with an updated patch, someone would be happy to
commit it for you.
best
vedant
> On May 11, 2016, at 10:01 AM, Mads Ravn via cfe-commits
> wrote:
>
> Hi,
>
> I would like to submit a patch for
> https://llvm.org/bugs/show_bug.cgi?id=27400 .
>
> Beside attaching the patch, is there anything I should be aware of? I have
> not submitted a patch before.
>
> You can find the patch attached to this mail.
>
> Kind regards,
> Mads Ravn
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
