Skip to content

More specific errors. - #598

Merged
TheDan64 merged 11 commits into
TheDan64:masterfrom
ErisianArchitect:master
Sep 3, 2025
Merged

More specific errors.#598
TheDan64 merged 11 commits into
TheDan64:masterfrom
ErisianArchitect:master

Conversation

@ErisianArchitect

Copy link
Copy Markdown
Contributor

Description

Many of the result types returned &'static str, or otherwise a wrapper around &'static str. I tried to find as many instances of this as I could to make the errors more specific. I may have missed some things.

Related Issue

#595

How This Has Been Tested

Unfortunately, I'm unable to test it because the tests environment won't compile on my machine due to POSIX-only functionality.

#597

Option<Breaking Changes>

I felt that it was a good idea for errors to be more specific so that it would be easier to determine the source of the error.

Checklist

@ErisianArchitect

Copy link
Copy Markdown
Contributor Author

It seems that there are some tests failing due to my change to is_alignment_ok, removing the ability for 0 alignment. As far as I'm aware, 0 alignment should be invalid, so I think perhaps these tests should be removed.

@ErisianArchitect

ErisianArchitect commented Aug 17, 2025

Copy link
Copy Markdown
Contributor Author

I looked into it some more, and I found this issue with this comment by one of the main contributors:

The "A value of 0 or" part of the docs is indeed incorrect. Zero is not a legal alignment.

Since 0 is not a legal alignment, I'm going to make the assumption that it's okay to remove these tests that check for an alignment of 0.

@ErisianArchitect

ErisianArchitect commented Aug 17, 2025

Copy link
Copy Markdown
Contributor Author

I decided to do a test to see what would happen if you tried to set the alignment to 0 just to make sure that it really wasn't a legal value, and this was the result when I tried to build the module:

thread 'main' panicked at src\bin\defer_test\main.rs:567:25:
Module verification failed.: "huge alignment values are unsupported\n  %defer_stack = alloca inalloca swifterror { [5 x ptr], i64 }, align 9223372036854775808\n"

So it seems that setting the alignment to 0 can cause undesirable outcomes, so I think it's a good thing that I removed the ability for alignments to be 0.

Edit: This is on inkwell 0.6.0 with LLVM 18.1.

Comment thread src/builder.rs Outdated
ExtractOutOfRange,
#[error("Bitwidth of a value is incorrect")]
BitwidthError(&'static str),
#[error("The bitwidth of value must be a power of 2 and greater than 8.")]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Isn't this >= 8?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just copied that message from the message that was passed into the BitwidthError, so if it's wrong then it was wrong before.

@TheDan64 TheDan64 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good; just have one question to clarify before merging

@ErisianArchitect

Copy link
Copy Markdown
Contributor Author

Any update on this?

@TheDan64
TheDan64 merged commit 7077b70 into TheDan64:master Sep 3, 2025
15 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