[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:492
+
+#define MACRO1 struct InMacro1 { int i; InMacro1() { i = 0; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'i' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer]
----------------
alexfh wrote:
> baloghadamsoftware wrote:
> > alexfh wrote:
> > > Could you add tests where the field name comes from a macro argument and from token pasting? Something along the lines of:
> > > 
> > > ```
> > > #define MACRO4(field) struct InMacro1 { int field; InMacro1() { field = 0; } }
> > > 
> > > MACRO4(q);
> > > 
> > > #define MACRO5(X) X
> > > 
> > > MACRO5(struct InMacro1 { int field; InMacro1() { field = 0; } });
> > > 
> > > #define MACRO6(field) struct InMacro1 { int qqq ## field; InMacro1() { qqq ## field = 0; } }
> > > 
> > > MACRO6(q);
> > > ```
> > It seems that handling of macro parameters in the `SourceManager` is totally off. The location in these cases are the macro call itself, but the spelling location is not the real location inside the macro, but in `MACRO4` it is the location of the argument still in the macro call. The case with `MACRO6` is even worse, because its spelling location is erroneous. So I could only added some fixmes. However, it does not crash, at least. That is the main intention of this particular patch.
> It's impossible to correctly handle replacements in all cases involving macros (apart from expanding all macros while applying fixes ;). The usual strategy is to warn always, but only suggest replacements when we can be sufficiently confident in their validity. In this case we can either disable fixes when any part of the code being touched is in a macro or try to detect that the whole range being modified is sort of "on the same level of macro hell". The `Lexer::makeFileCharRange` is supposed to help with the latter (see clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp for an example of using it).
Actually, this is what the patch does: it does not suggest a fix in complex macro cases, only in the simplest ones. And most importantly, it prevents the crash mentioned in the //Bugzilla// bug report.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89380/new/

https://reviews.llvm.org/D89380