Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] Patch for review - constexpr evaluation of __builtin_ffs

Hi all,


> On 16/5/22 11:52 pm, Davin McCall wrote:

for some reason I didn't receive this mail, I hope this is just an issue with my inbox.


> I was hoping that someone with more appropriate knowledge and experience than me may step up on this

Disclaimer: I don't claim to have appropriate knowledge and experience ;)


I agree wit Davins "seems to work" statement, because I could successfully test the fix. I think the general approach of adding ICPPExecutions to GCCBuiltinSymbolProvider should be okay. I also agree that a general builtin implementation of ICPPExecution would be desirable.

I added a test case to org.eclipse.cdt.core.parser.tests.ast2.cxx14.constexpr.FunctionTests and the test fails. This is due to the missing marshalling implementation of BuiltinFfs::marshal(...).


Some regression tests fail because the patch adds getFunctionBodyExecution to the CPPImplicitFunction class which overrides its super implementation. This works for the builtin function but in other cases (e.g. lambdas) the super implementation must be called.


BR,

Dominic


On 5/20/22 20:50, Jonah Graham wrote:
> It'd be appreciated if someone with appropriate knowledge and experience would have a look at the patch:

@all Can someone who feels comfortable in this code have a look at this please?

I was hoping that someone with more appropriate knowledge and experience than me may step up on this - if not I will try to understand this part of the code so we can move forward on this.

Thanks
Jonah



~~~
Jonah Graham
Kichwa Coders
www.kichwacoders.com


On Tue, 17 May 2022 at 07:38, Davin McCall <davmac@xxxxxxxxxx> wrote:

I have recently found some time again to work on a CDT issue that was affecting one of my projects.

Having sat on this for a little, it's occurred to me that having BuiltinFffs implement ICPPExecution and not handle marshal() might be problematic. I guess it could be better to have a general "Builtin" implementation of ICPPExecution which can marshal. It could be extended to handle other builtins later.

However, I'm not overly familiar with how these things fit together and would still love to get some guidance / feedback.

Thanks,

Davin

On 16/5/22 11:52 pm, Davin McCall wrote:

Hi cdt-dev,

I'm a long-time lurker on this list, though I have made a few minor contributions in the past.

I have recently found some time again to work on a CDT issue that was affecting one of my projects. As detailed in https://bugs.eclipse.org/bugs/show_bug.cgi?id=579934 :

The following code gives "invalid template arguments" on the var declaration:

--- begin ---
template <unsigned V>
class TT
{

};

TT<__builtin_ffs(4)> var;
--- end ---

I have a fix for this though I'm a little uncertain of the approach. It "seems to work" but unfortunately the unit tests don't seem to correctly recognize builtin functions and so I couldn't add a test (I tried to put one in org.eclipse.cdt.core.parser.tests.ast2.cxx14.constexpr.IntegralValueTests, but not luck as I mentioned).

It'd be appreciated if someone with appropriate knowledge and experience would have a look at the patch:

    https://git.eclipse.org/r/c/cdt/org.eclipse.cdt/+/193395

Regards,

Davin



_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdt-dev
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdt-dev

_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/cdt-dev

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Back to the top