Factor this out into a visitConditional() that calls the given visitor function.
Details
Diff Detail
Event Timeline
clang/lib/AST/Interp/ByteCodeExprGen.cpp | ||
---|---|---|
920 | This meant to be here? | |
921 | Is this condition supposed to be just a 'visit', or should this be using the functor? | |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
200 | I'd probably rather make this a template taking a functor, rather than bringing in the atrocities that come with std::function. | |
clang/test/AST/Interp/records.cpp | ||
339 | Did this not work before? I don't see what part of teh code changes makes this work? |
clang/lib/AST/Interp/ByteCodeExprGen.cpp | ||
---|---|---|
920 | Nope! :( | |
921 | The visit is correct here, the functor is just for the true/false expr. | |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
200 | Alright, will look into that | |
clang/test/AST/Interp/records.cpp | ||
339 | We call visitInitializer also when the return value is constructed in the calling function, so the return statement in getS() exercises the code added. |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
---|---|---|
200 | Done. Not sure if this is exactly how you meant it to look. Feels a bit inelegant. |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
---|---|---|
200 | This actually probably needs to be a non-member now, since its only used in the .cpp. while I DO dislike that it was using std::function (probably because of captures in the lambdas?), I now suspect this is worse if we can't have it in the header, right? |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
---|---|---|
199–200 | The template definition isn't available within the header file, so this is fragile (impossible to instantiate from anything but ByteCodeExprGen.cpp). |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
---|---|---|
199–200 | What's the alternative? If I make it a static function in ByteCodeExprGen.cpp, I'd have to make a lot of members of ByteCodeEmitter etc. public which is not a very clean solution. Moving the definition into the header file isn't very nice either, all the others are in the source file. |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
---|---|---|
199–200 | What about making it a static template in ByteCodeExprGen.cpp, but make it a 'friend' here? I think that would work, wouldn't it? Something like: https://godbolt.org/z/ofYbGvfYa You unfortunately cannot make it static, but as it is a template, at least it is inline. WDYT? |
clang/lib/AST/Interp/ByteCodeExprGen.h | ||
---|---|---|
199–200 | I tried that but I keep running into linker problems: mold: error: undefined symbol: lib/libclangAST.a(ByteCodeExprGen.cpp.o): bool clang::interp::visitConditional<clang::interp::ByteCodeExprGen<clang::interp::ByteCodeEmitter>::VisitAbstractConditionalOperator(clang::AbstractConditionalOperator const*)::{lambda(clang::Expr const*)#1}>(clang::interp::ByteCodeExprGen<clang::interp::ByteCodeEmitter>*, clang::AbstractConditionalOperator const*, clang::interp::ByteCodeExprGen<clang::interp::ByteCodeEmitter>::VisitAbstractConditionalOperator(clang::AbstractConditionalOperator const*)::{lambda(clang::Expr const*)#1}) (and another one for EvalEmitter). But I can't investigate more before Wednesday. |
I'd probably rather make this a template taking a functor, rather than bringing in the atrocities that come with std::function.