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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

replace-dependent-typedef breaks on typedef in template struct #279

Open
emaxx-google opened this issue Jan 14, 2025 · 4 comments · May be fixed by #281
Open

replace-dependent-typedef breaks on typedef in template struct #279

emaxx-google opened this issue Jan 14, 2025 · 4 comments · May be fixed by #281
Assignees
@chenyang78
Labels
bug

Comments

@emaxx-google
Copy link

The following error is generated:

pass_clang::replace-dependent-typedef has encountered a bug:
pass failed to modify the variant

when the input is:

template <typename T>
struct A {
  struct Inner;
  typedef A::Inner Bar;
};
@emaxx-google
Copy link
Author

It seems that the problem is this check that gets misled by different string representations: A<T>::Inner vs. A::Inner:

creduce/clang_delta/ReplaceDependentTypedef.cpp

Line 148 in 31e855e

if (Str == TdefTyStr)

@emaxx-google
Copy link
Author

emaxx-google commented Jan 14, 2025 •
edited
Loading

Some possible directions:

  • A: Do string comparison of the original text and the replacement - and exit with an error if it's the same.
    • => very simple to implement, but feels a bit wasteful (2 applications of the pass in the same location might be required before the idempotence gets discovered).
  • B: Skip applying the pass to typedefs that don't actually refer, directly or indirectly, to another typedef.
    • => this corresponds better to the idea behind the pass (the word "dependent" in the pass' name), but might be a bit more fragile to implement.
  • ...?

@eeide eeide added the bug label Jan 14, 2025
@eeide
Copy link
Member

eeide commented Jan 14, 2025

Thank you for your bug report!

@emaxx-google
Copy link
Author

I'm happy to propose a PR as a starting point. (And probably basic unit tests for this pass.)

@emaxx-google emaxx-google mentioned this issue Jan 14, 2025
Open
emaxx-google added a commit to emaxx-google/creduce that referenced this issue Jan 14, 2025
@emaxx-google
Skip non-dep typedefs in replace-dependent-typedef
b6c7130 
Only consider typedefs that depend on other typedefs in
replace-dependent-typedef.

This prevents the pass from reporting a success when no actual
change has been made, like in the case when a typedef belongs to
and refers to a template struct (where multiple possible string
representations may confuse the checks).

This fixes csmith-project#279.
emaxx-google added a commit to emaxx-google/creduce that referenced this issue Jan 14, 2025
@emaxx-google
Skip non-dep typedefs in replace-dependent-typedef
d0f4d96 
Only consider typedefs that depend on other typedefs in
replace-dependent-typedef.

This prevents the pass from reporting a success when no actual
change has been made, like in the case when a typedef belongs to
and refers to a template struct (where multiple possible string
representations may confuse the checks).

This fixes csmith-project#279.
emaxx-google added a commit to emaxx-google/creduce that referenced this issue Jan 14, 2025
@emaxx-google
Skip non-dep typedefs in replace-dependent-typedef
c0f4192 
Only consider typedefs that depend on other typedefs in
replace-dependent-typedef.

This prevents the pass from reporting a success when no actual
change has been made, like in the case when a typedef belongs to
and refers to a template struct (where multiple possible string
representations may confuse the checks).

This fixes csmith-project#279.
@emaxx-google emaxx-google linked a pull request Jan 14, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chenyang78 chenyang78

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Skip non-dep typedefs in replace-dependent-typedef emaxx-google/creduce
3 participants
@eeide @chenyang78 @emaxx-google