Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors in transforming struct T {} var #588

Closed
5 tasks
Ukilele opened this issue Sep 29, 2023 · 2 comments
Closed
5 tasks

Errors in transforming struct T {} var #588

Ukilele opened this issue Sep 29, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@Ukilele
Copy link

Ukilele commented Sep 29, 2023

Hey Andreas, I am sorry that I report so many issues recently, but cppinsights is just a great tool for learning C++ and it seems I occasionally tend to dig into the slightly more odd edge cases of the language 馃樃

Here is the source code:

struct A {
  void f() {}
} a_var_1;

struct B {
} b_var_1, b_var_2, b_var_3;

struct {
} anonym_var_1, anonym_var_2, anonym_var_3;

struct {
} anonym_var_assign = {};

struct {
} anonym_var_array[3];

struct {
} anonym_var_dummy, *anonym_var_ptr;

And this is what cppinsights (version 26c63ad) produces:

struct A a_var_1;


struct B b_var_3;


struct __anon_8_1
{
  // inline constexpr __anon_8_1() noexcept = default;
};

__anon_8_1 anonym_var_1;
, anonym_var_2__anon_8_1 anonym_var_2;
, anonym_var_3;__anon_8_1 anonym_var_3;


struct __anon_11_1
{
};

 = {};__anon_11_1 anonym_var_assign = {};


__anon_14_1 anonym_var_array[3];


__anon_17_1 * anonym_var_ptr;

I think there are several issues, but I assume they are related. So I hope it's fine that I create just one issue.

  • 1. Creating a type and variable via struct A{} a_var_1; produces struct A a_var_1;. But instead it should show the whole type definition and a variable definition afterwards:
struct A {
  //...
};
A a_var_1;
  • 2. Creating a type and multiple variables via struct B {} b_var_1, b_var_2, b_var_3; produces struct B b_var_3; (similar like in bullet point 1.). The additional issue is, that cppinsights completely swallows the definitions of b_var_1 and b_var_2.
  • 3. Creating an anonymous type and multiple variables via struct {} anonym_var_1, anonym_var_2, anonym_var_3; does not swallow the first and second variables (other than in bullet point 2.), but only the definition of anonym_var_1 looks reasonable. The definitions of anonym_var_2 and anonym_var_3 look wrong.
  • 4. Creating an anonymous type and one variable which gets initialized like in struct {} anonym_var_assign = {}; looks wrong in a sense that the initialization part ( = {};) appears both before and after the variable anonym_var_assign.
  • 5. Creating an anonymous type and one or multiple variables with additional declarators (like array ([3]) or pointer (*)) leads to the effect that both the anonymous struct definition does not get generated by cppinsights and that previous variable definitions (anonym_var_dummy) get swallowed.
@andreasfertig
Copy link
Owner

Hello @Ukilele,

no worries, you help to improve C++ Insights, thank you for that :-).

As far as I could see, all the issues you report have one common root. When more than one variable is declared per line, or a type is defined in the same line, the AST matchers I use from Clang trigger on that line multiple times. What you then see is only the last rewrite that survives. A related issue (#500) reports the root of the trouble: int a,b,c also fails. Same reason. I don't know how to fix this with the current implementation of C++ Insights.

However, a way to fix this is to eliminate all the matches and traverse the TU. I have plans for that because this approach would also drop pounds of code I must maintain. The problem with the TU traverser is that I have to deal with includes that are not part of the AST. I can get them, but positioning them is an issue. By coincidence, I started looking at the TU approach this week. But I'm not sure whether I will make progress soon.

One easy fix to see the transformation is to wrap your code in a namespace. That way, the namespace matcher picks it up and rewrites it correctly.

Andreas

@andreasfertig andreasfertig added the bug Something isn't working label Sep 29, 2023
@Ukilele
Copy link
Author

Ukilele commented Sep 29, 2023

Thanks for your quick response :)
And thanks for sharing the insights of cppinsights (pun intended).
The namespace-workaround sounds good 馃憤

andreasfertig added a commit that referenced this issue Mar 11, 2024
This change introduces a `TranslationUnit` handler, replacing the previous
matchers. The advantage is that _all_ statements are processed,  not only the
ones where an exact match exists.

The new approach fixes #376, #500, and #588.

There is one disadvantage: for primary templates, some code is not reproduced
as written. This causes one compile issue.
andreasfertig added a commit that referenced this issue Mar 11, 2024
This change introduces a `TranslationUnit` handler, replacing the previous
matchers. The advantage is that _all_ statements are processed,  not only the
ones where an exact match exists.

The new approach fixes #376, #500, and #588.

There is one disadvantage: for primary templates, some code is not reproduced
as written. This causes one compile issue.
andreasfertig added a commit that referenced this issue Mar 11, 2024
This change introduces a `TranslationUnit` handler, replacing the previous
matchers. The advantage is that _all_ statements are processed,  not only the
ones where an exact match exists.

The new approach fixes #376, #500, and #588.

There is one disadvantage: for primary templates, some code is not reproduced
as written. This causes one compile issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants