[RELAX] Improve well-formed checker error messages with function names#18791
Conversation
Summary of ChangesHello @guan404ming, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the debugging experience for Relax IR by enhancing the well-formed checker's error messages. Previously, errors related to duplicate variables across functions lacked specific function context. The changes introduce a mechanism to map function pointers to their names and integrate these names into the error messages for duplicate parameters and symbolic variables, providing clearer and more actionable feedback to developers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
6da4c5d to
7bd91b2
Compare
There was a problem hiding this comment.
Code Review
This pull request improves the error messages in the well-formed checker by including function names when reporting duplicate variable usage across functions. This is a valuable enhancement for debugging. The implementation is straightforward and correct. I have one minor suggestion to optimize a map lookup in an error-reporting path.
src/relax/analysis/well_formed.cc
Outdated
| if (param_var_func_map_.count(param) == 1) { | ||
| // TODO(relax-team): Complete this error info after we integrate printer | ||
| Malformed(Diagnostic::Error(param->span) | ||
| << "Relax variable " << param | ||
| << " is repeatedly used as parameters in function."); | ||
| << "Relax variable " << param << " is used as a parameter in both function " | ||
| << FuncName(param_var_func_map_.at(param)) << " and function " | ||
| << FuncName(cur_visited_func_) << "."); | ||
| } |
There was a problem hiding this comment.
To avoid a double lookup in param_var_func_map_, you can use find once and then use the resulting iterator. This is slightly more efficient and a common C++ pattern.
| if (param_var_func_map_.count(param) == 1) { | |
| // TODO(relax-team): Complete this error info after we integrate printer | |
| Malformed(Diagnostic::Error(param->span) | |
| << "Relax variable " << param | |
| << " is repeatedly used as parameters in function."); | |
| << "Relax variable " << param << " is used as a parameter in both function " | |
| << FuncName(param_var_func_map_.at(param)) << " and function " | |
| << FuncName(cur_visited_func_) << "."); | |
| } | |
| auto it = param_var_func_map_.find(param); | |
| if (it != param_var_func_map_.end()) { | |
| Malformed(Diagnostic::Error(param->span) | |
| << "Relax variable " << param << " is used as a parameter in both function " | |
| << FuncName(it->second) << " and function " | |
| << FuncName(cur_visited_func_) << "."); | |
| } |
484eeb7 to
8bc1f55
Compare
Signed-off-by: Guan-Ming Chiu <guanmingchiu@gmail.com>
8bc1f55 to
af9f177
Compare
|
Thank you! @guan404ming |
Why
Error messages for duplicate variables across functions lacked context about which functions were involved, making debugging difficult.
How