https://github.com/AaronBallman closed
https://github.com/llvm/llvm-project/pull/87792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/87792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Great! Nothing else from my side
https://github.com/llvm/llvm-project/pull/87792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/revane updated
https://github.com/llvm/llvm-project/pull/87792
>From c354f7f0be526fe41a56198c5d0ca434a8ad0bfe Mon Sep 17 00:00:00 2001
From: Edwin Vane
Date: Fri, 5 Apr 2024 11:06:01 -0400
Subject: [PATCH] [clang-tidy] Allow renaming macro arguments
Although the identifier-n
revane wrote:
Already done. Unless you had something else in mind?
https://github.com/llvm/llvm-project/pull/87792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlosgalvezp wrote:
Nice! Should we add this example as a test case?
https://github.com/llvm/llvm-project/pull/87792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
AaronBallman wrote:
> Using the suggested code and this one usage:
>
> ```
> MY_MACRO(myglob);
> ```
>
> No suggestion is made. My understanding is because the full range of the
> NamedDecl would contain `awesome_myglob` which isn't entirely within a macro
> arg.
Oh, awesome, thank you for t
https://github.com/revane updated
https://github.com/llvm/llvm-project/pull/87792
>From b34156654bfa937b180eaad1061e38c10a799235 Mon Sep 17 00:00:00 2001
From: Edwin Vane
Date: Fri, 5 Apr 2024 11:06:01 -0400
Subject: [PATCH] [clang-tidy] Allow renaming macro arguments
Although the identifier-n
revane wrote:
Using the suggested code and this one usage:
```
MY_MACRO(myglob);
```
No suggestion is made. My understanding is because the full range of the
NamedDecl would contain `awesome_myglob` which isn't entirely within a macro
arg.
https://github.com/llvm/llvm-project/pull/87792
__
AaronBallman wrote:
> > the user may not have the ability to change the definition of the macro to
> > be able to appease the check
>
> My understanding of this PR is that the user would only need to change what
> they pass into the macro, not the macro itself, or? E.g.
>
> ```c++
> -MY_MACRO
revane wrote:
Indeed. I can add some tests to make sure nested macros aren't affected but my
thought was since this change allows renaming _arguments_ (i.e. not the macro
parameters) then it was safe since the code is not really "inside a macro".
There are already other means to filter out sys
carlosgalvezp wrote:
> the user may not have the ability to change the definition of the macro to be
> able to appease the check
My understanding of this PR is that the user would only need to change what
they pass into the macro, not the macro itself, or? E.g.
```cpp
-MY_MACRO(foo)
+MY_MACRO
https://github.com/AaronBallman commented:
I am not certain we want to allow this without some kind of option. The trouble
is: the user may not have the ability to change the definition of the macro to
be able to appease the check. However, the clang-tidy folks may have different
opinions/idea
revane wrote:
@AaronBallman What do you think?
https://github.com/llvm/llvm-project/pull/87792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
revane wrote:
Macros are tricky business and I understand why renaming is not allowed within
macros. For the cases in the lit test, the renaming seems safe. Is this
generally the case (i.e. those permitted by
`clang::tidy::utils::rangeCanBeFixed()`)? What other test cases can we add? Or
is th
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Edwin Vane (revane)
Changes
Although the identifier-naming.cpp lit test expected macro arguments not to be
renamed, the code seemed to already allow it. The code was simply not being
exercised because a SourceManager argument wasn't
https://github.com/revane created
https://github.com/llvm/llvm-project/pull/87792
Although the identifier-naming.cpp lit test expected macro arguments not to be
renamed, the code seemed to already allow it. The code was simply not being
exercised because a SourceManager argument wasn't being p
17 matches
Mail list logo
