Fix compilation for visual studio clang distribution #891
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| Valuable::universal_lambda_t CompileIntoLambda(Valuable::variables_for_lambda_t) const override { | ||
| return [](Valuable::universal_lambda_params_t) -> Valuable { | ||
| return GlobalObject; | ||
| return NewDefaultValuable(); |
There was a problem hiding this comment.
The PR introduces a template-related issue in Constant.h that's causing build failures. The error is: "there are no arguments to 'NewDefaultValuable' that depend on a template parameter, so a declaration of 'NewDefaultValuable' must be available [-fpermissive]"
The problem is that NewDefaultValuable() is being called in Constant.h before it's declared in the template context. To fix this, you need to either:
- Forward-declare the
NewDefaultValuable()method before using it inCompileIntoLambda() - Or move the implementation of
NewDefaultValuable()inValuableDescendantContract.hbefore theCompileIntoLambda()method inConstant.h
This is causing build failures on both Ubuntu and Windows CI environments.
| Valuable Euler::Sq() const { | ||
| auto exponentiation = ptrs::make_shared<Exponentiation>(*this, 2); | ||
| return Valuable(exponentiation); | ||
| return Valuable(std::move(exponentiation)); |
There was a problem hiding this comment.
The std::move() addition in Euler.cpp is a good optimization for avoiding unnecessary copies of shared pointers. However, for consistency, you might want to apply similar optimizations in other similar code patterns throughout the codebase. This would help with both performance and ensuring consistent coding style across the project.
|
|
||
| static constexpr Valuable NewDefaultValuable() { | ||
| auto obj = ptrs::make_shared<Chld>(); | ||
| return Valuable(std::static_pointer_cast<Valuable>(obj)); |
There was a problem hiding this comment.
The new NewDefaultValuable() method in ValuableDescendantContract.h is a good addition for creating default valuable objects, but it's causing template resolution issues when used in Constant.h. Consider adding a forward declaration or rearranging the code to ensure the method is visible before it's used.
Also, since this is a static method that creates a new object, consider adding a brief comment explaining its purpose and when it should be used versus the existing GlobalObject pattern.
ohhmm
force-pushed
the
vs-clang-fix
branch
3 times, most recently
from
f5f0854 to
8198844
Compare
ohhmm
force-pushed
the
vs-clang-fix
branch
2 times, most recently
from
8c775c1 to
22d469d
Compare
| return exp; | ||
| } | ||
|
|
||
| static constexpr Valuable NewDefaultValuable() { |
There was a problem hiding this comment.
The NewDefaultValuable() method in ValuableDescendantContract.h is defined after it's used in Constant.h, which could cause template resolution issues with some compilers (particularly Visual Studio's Clang distribution).
Recommendation: Consider moving the NewDefaultValuable() method definition earlier in the ValuableDescendantContract class, before it's used in CompileIntoLambda(), or add a forward declaration at the beginning of the class.
| Valuable Euler::Sq() const { | ||
| auto exponentiation = ptrs::make_shared<Exponentiation>(*this, 2); | ||
| return Valuable(exponentiation); | ||
| return Valuable(std::move(exponentiation)); |
There was a problem hiding this comment.
The std::move() optimization in Euler.cpp is a good improvement for avoiding unnecessary copies of shared pointers. For consistency, consider applying similar optimizations in other similar code patterns throughout the codebase, such as in other classes that return Valuable objects with shared pointers.
| return exp; | ||
| } | ||
|
|
||
| static constexpr Valuable NewDefaultValuable() { |
There was a problem hiding this comment.
The new NewDefaultValuable() method in ValuableDescendantContract.h lacks documentation explaining its purpose and when it should be used versus the existing GlobalObject pattern. Consider adding a brief comment to clarify its intended usage and relationship to other object creation methods in the codebase.
ohhmm
force-pushed
the
vs-clang-fix
branch
3 times, most recently
from
f317aa1 to
f0d5393
Compare
Fix compilation for visual studio clang distribution