zeyi2 wrote: I believe this PR needs some explanation, so here is my rationale:
## The idea behind the change The original implementation relies on string concatenation and manual source code slicing. To accurately handle every possible edge case this way, one would essentially need to re-implement a significant part of a C parser, which is error-prone. Since the C++ compiler already has full knowledge of these types, my proposed approach is to retrieve this information directly from AST. ## So why keep the original logic? Macros.. AST printing expands macro information, which is not the intended behavior for this check and can lead to unwanted results. I decided to handle the problem this way: only use the compiler-provided type strings when we are certain no macros are involved; otherwise, fall back to the original text-based logic. ## Why not cleanup the original logic now? While the original logic can definitely be simplified, I prefer to leave that for a separate PR. This PR already introduces significant (testcases) changes that might be controversial. IMHO adding cleanup on top of it would only make the review process more difficult :) ## The tradeoffs. **Pros:** - reduces the amount of generation of invalid fix-its. - supports extremely complex declarations that were previously unhandled. **Cons** - requires modifications to existing test cases. but since the new declarations are generated by Clang itself, I think they are semantically correct, so maybe acceptable? - lose some "aliased" type definitions in fixes (Example: a fix like using `Function = bool (*)(PointType, PointType);` might now be expanded to the underlying types `using Function = bool (*)(double *, double *);`) --- Feedback of any form is welcomed :) Thanks in advance! https://github.com/llvm/llvm-project/pull/174104 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
