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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SymbolExternalizer: Rename externalized operand of sizeof #47

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

marcosps
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@giulianobelinassi giulianobelinassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the comments in my review.

* }
*/
DeclStmt *dstmt = (DeclStmt *) stmt;
Decl *ddecl = dstmt->getSingleDecl();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should do an assertion here to check if the type of this Decl is an array of some sort by using ddecl->getType()->isArrayType() just to be sure that we are not overwriting something we shouldn't.

auto vec_of_ranges = Get_Range_Of_Identifier_In_SrcRange(ddecl->getSourceRange(),
OldSymbolName.c_str());

if (vec_of_ranges.size() > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may iterate on all elements of vec_of_ranges because we can have things like char y[sizeof(x)][sizeof(x)].


void pu_f(void)
{
char s[sizeof(ee_o)];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add two checks:

  • char t[sizeof(ee_o)][sizeof(ee_o)];
  • The new sizeof matches the sizeof with the old declaration. You can do that by declaring an auxiliary array: char v[sizeof(x) - 4] for example. Any array size >= 0 will be accepted by the compiler and thus you can implement an static_assert with it. Or perhaps just do straight _Static_assert().

@marcosps
Copy link
Collaborator Author

I added static_assert and checked for x[sizeof(y)][sizeof(y)], and it worked. The only thing that was not working was the assert for the arrayType.

@giulianobelinassi
Copy link
Collaborator

Added an issue in llvm about the fact that ConstantArrayType does not have a size expr in this case.

llvm/llvm-project#95584

@giulianobelinassi giulianobelinassi merged commit 8376ab2 into SUSE:main Jun 14, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants