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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
