baloghadamsoftware marked an inline comment as done. baloghadamsoftware added a comment.
In D89380#2330076 <https://reviews.llvm.org/D89380#2330076>, @alexfh wrote: > Thanks for the fix! However, I'm not sure it's possible to correctly rewrite > code in all cases where macros are involved. See a couple of motivating > examples in the comment. ================ 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: > 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. 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
