whisperity marked 4 inline comments as done.
whisperity added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:255
+ std::string NodeTypeName =
+ Node->getType().getAsString(Node->getASTContext().getPrintingPolicy());
+ StringRef CaptureName{NodeTypeName};
----------------
whisperity wrote:
> aaron.ballman wrote:
> > whisperity wrote:
> > > aaron.ballman wrote:
> > > > Hmm, one downside to using the printing policy to get the node name is
> > > > that there can be alternative spellings for the same type that the user
> > > > might reasonably expect to be applied. e.g., if the user expects to
> > > > ignore `bool` datatypes and the printing policy wants to spell the type
> > > > as `_Bool`, this won't behave as expected.
> > > But then the diagnostic is inconsistent with what the compiler is
> > > configured to output as diagnostics, isn't it? Can I stringify through
> > > Clang with some "default" printing policy?
> > The situation I'm worried about is something like this in C code:
> > ```
> > #include <stdbool.h>
> >
> > void func(bool b, bool c) {}
> > ```
> > because the `bool` in the function signature will expand to `_Bool` after
> > the preprocessor gets to it (because of the contents of `<stdbool.h>`). If
> > the user writes `bool` in their configuration file because that's what
> > they've written in their function signatures, will this fail because the
> > printing policy says it should be `_Bool`?
> >
> > Along similar lines, I wonder about things like `struct S` vs `S` (in C),
> > `signed char` vs `char`, types within anonymous namespaces (where we print
> > `<anonymous namespace>`), etc. Basically, any time when the printing policy
> > may differ from what the user lexically writes in code.
> Meanwhile, I realised we are talking of two entirely distinct things. I will
> look into how this `PrintingPolicy` stuff works... I agree that the ignore
> rules (where the comment was originally posted) should respect the text
> written into the code as-is. The diagnostics printing type names could
> continue using the proper printing policy (to be in line with how Sema
> diagnoses, AFAIK).
Right, @aaron.ballman I think I got this tested. Please check the C test file.
`bool` becomes `_Bool` in C code by the preprocessor, but the printing policy
still says both should be printed as `bool`. I added some test cases about
this. It is weird, because without doing some extra magic specifically against
macros (which might break detecting type names in other locations), I can't
really test ignoring `_Bool` but not ignoring `bool`. Now, the diagnostics
respect the printing policy, but the type names are taken with an empty
printing policy, which //should// give us the result as-is in the code.
Unfortunately, "as-is" still means `_Bool` for `bool`'s case, so even if you
ignore `bool`, you will still get results where `bool` is written, because
`bool` is a macro (Tidy automatically puts a note about this!) and the real
type is `_Bool`, as per the macro expansion.
Wow... okay, that explanation got convoluted.
Anyways, it's //really weird//. But: we no longer take the actual printing
policy anymore, but the default, which should eliminate all these cases.
The `bool`/`_Bool` case doesn't fail because of the printing policy, it fails
because `bool` is a macro in `<stdbool.h>`.
Also, the type name ignore list not a full match, but a //substring match//
ignore list. So if you have a type named `struct Foo`, if you ignore `Foo`,
both `struct Foo`, `Foo`, `MyFoo`, etc. will be also ignored. And it's case
sensitive.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69560/new/
https://reviews.llvm.org/D69560
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
