[MISC] Fix compile warnings #16571
[MISC] Fix compile warnings #16571
Conversation
This PR fixes the following compile warnings: ``` [build] warning: unknown command tag name [-Wdocumentation-unknown-command] [build] 90 | * @R.function [build] | ^~ [build] warning: 'tvm::codegen::CodeGenWebGPU::AddFunction' hides overloaded virtual function [-Woverloaded-virtual] [build] 52 | runtime::FunctionInfo AddFunction(const PrimFunc& f, bool skip_readonly_decl); // NOLINT(*) [build] | ```
Lunderberg
left a comment
Lunderberg
left a comment
There was a problem hiding this comment.
Overall, looks good, and thank you for the fix. I have one question about a warning that should perhaps be fixed in a different manner, but that can be in a follow-up PR.
| explicit CodeGenWebGPU(Target target); | ||
| // overrides | ||
| std::string Finish() final; | ||
| using CodeGenC::AddFunction; |
There was a problem hiding this comment.
I assume this is for silencing a warning about shadowing CodeGenC::AddFunction? If so, could we instead remove the skip_readonly_decl argument altogether?
The CodeGenWebGPU::AddFunction method is only ever called here, and it is always passed false. I'm wondering if it should be part of the CodeGenWebGPU constructor instead.
There was a problem hiding this comment.
Thanks for your comment, but I'm not sure if it helps. As stack overflow says, the warning happens if the functions have the same names but different signatures.
Indeed, there are different solutions, like renaming CodeGenWebGPU::AddFunction. Meanwhile, I think it's also fine to use using CodeGenC::AddFunction;, as the compiler will import the function automatically and implicitly.
I have no personal preference. I would love to follow up if you have a strong opinion on a specific solution :)
There was a problem hiding this comment.
My main concern is because CodeGenC::AddFunction is a virtual function. The CodeGenWebGPU::AddFunction shadows CodeGenC::AddFunction, but doesn't override it due to the different signature. Calls made to this function through a base class pointer CodegenC* my_codegen = get_codegen(); my_codegen->AddFunction(...) would therefore call CodeGenC::AddFunction not CodeGenWebGPU::AddFunction.
I don't think there are any calls to AddFunction through a base class pointer, but its something that could cause unexpected breakage in the future.
4 participants
This PR fixes the following compile warnings with clang-17:
cc @Lunderberg @tqchen