implicit cast after explicit cast

[viboes]

Firts of all, thanks for working on this awesone tool.
Hopping I could be able to submit patches in a no so far future

As for https://cppinsights.io/s/b16e6b9b there is an erroneous implicit conversion added after an explicit one.

  float f = (float)(3.0);

after

  float f = static_cast<float>(static_cast<float>((3.0)));

[andreasfertig]

Hello @viboes,

my pleasure!
Looking forward to patches, they are always welcome.

Thank for reporting this, tt is not the first time that came up (#134). If you look at the AST you can see, that there is a CStyleCastExpr and as a direct descended ImplicitCastExpr. That's why there are two static_cast. However, having looked at it one more time, due to your report. It looks to me a bit like that Clang already knows, that the final type is float and not double. This I believe is the ImplicitCastExpr. Then you casting it explicitly to float with a C-style-cast which is the second static_cast. The ImplicitCastExpr stays even if you remove the static_cast. I assume Clang reduces the redundant casts to just one later in the stage.

That means, what's currently show is correct in terms that it is in the AST. However, it confuses people. I can try to filter the ImplicitCastExpr and show it only if -show-all-implicit-casts is enabled.

What do you think?

Andreas

[viboes]

Hi, I believe the fisrt thing could be to have a list of know limitations on the documentation so that we can avoid maybe creating a new github issue and woud explain what is going on.

I would need to take a look at the AST. I was wondering if this couldn't be an option for the tool to get alternatively th eAST so that the student makes the relations between the transformation and the AST. I'll create a enhancement issue. Up to the active community to address this feature.

I agree that the resut is correct, it is just confusing.

Having the option -show-all-implicit-casts could help to identify maybe which implicit feature is behind th escenes. Wondering if having an option for every transformation could be possible and if at all wouldn't it improve the tool.

[andreasfertig]

Hello @viboes,

in the meantime I found out about isPartOfExplicitCast. With that I can filter it. A fix is on its way.

However, I agree that a list with known limitation is a good idea. During each release a documentation is generated with doxygen and hosted here https://docs.cppinsights.io. My favorite approach would be to add a couple of markdown files in the doc folder and have doxygen generate something nice out of it. That way a user can generate and use the documentation offline.

I was wondering if this couldn't be an option for the tool to get alternatively th eAST so that the student makes the relations between the transformation and the AST.

That sounds very hard to me, depending on how close they should link together. Plus the main idea of C++ Insights is to show C++ code. Do you think that the AST is something a majority is interested in?

Andreas

[viboes]

How can I verify that this has been fixed. I was opening again the link and I git the same duplicated output .

[viboes]

For the AST we have clang-check which output the AST, isn't it. The idea was to switch between th etransformation and the AST.

[andreasfertig]

Hello @viboes,

the initial link you attached here has "show all implicit casts" enabled. With that it looks like before. Turn it of and you will see the difference and with that the fix. Here is a version with it turned of: https://cppinsights.io/s/1f7ea4cc.

For the AST we have clang-check which output the AST, isn't it. The idea was to switch between th etransformation and the AST.

As this issue is closed, may be open another one for this. I'm not sure I do understand what you have in mind.

Andreas

[viboes]

Ok, I missed the flag.