Michael Forney <mforney@mforney.org> wrote:
> ---
>  README.md | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/README.md b/README.md
> index 86ae666e..082dd943 100644
> --- a/README.md
> +++ b/README.md
> @@ -124,7 +124,7 @@ inspired to actually go out and do it when I saw [boson].  `muon`'s code was
>  originally based on `boson`, though has since been almost completely rewritten.
>  
>  [muon]: https://muon.build
> -[contributing guide]: https://git.sr.ht/~lattis/muon/tree/master/item/doc/contributing.md

Stone Tickle <lattis@mochiro.moe> wrote:
> This looks good to me.  Having to write (void)x is tiresome but I think
> it is worth it to keep -Wunused-variable on.  Since the problem only
> really arises in these custom iterators which require a place to put the
> key and value, I suppose one idea would be to make specialized macros
> for when you only want to iterate over the keys or values...

Another option might be to move them into the iterator macros, with
the assumption that if they weren't used in the body, they weren't
needed.

> Anyways, good luck getting cproc to work, hopefully it isn't difficult.

These were the only two patches needed. Thanks for sticking to

---
 README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 86ae666e..082dd943 100644
--- a/README.md
+++ b/README.md
@@ -124,7 +124,7 @@ inspired to actually go out and do it when I saw [boson].  `muon`'s code was
 originally based on `boson`, though has since been almost completely rewritten.
 
 [muon]: https://muon.build
-[contributing guide]: https://git.sr.ht/~lattis/muon/tree/master/item/doc/contributing.md
+[contributing guide]: https://docs.muon.build/contributing.html
[message trimmed]

Scott Graham <sgraham@gmail.com> wrote:
> It looks like this
> 
> https://c9x.me/git/qbe.git/tree/ssa.c?id=327736b3a6e77b201fffebd3cc207662e823d3a5#n269
> 
> should probably be err()ing. But it doesn't have a lot of context for
> a message, so maybe its callers should instead.

I don't think we can error here. While the reproducer and the
original program did have uninitialized reads, it's possible to
construct a program which doesn't and still triggers the assertion.

Here's another example (C code):

Stone Tickle <lattis@mochiro.moe> wrote:
> Nice catch.  By the way, did you find the output of this subcommand
> useful?  C build tools (unfortunately) need to maintain some sort of
> database of compiler information and this was a small experiment to try
> and make muon's compiler database more transparent.  I'd like to also
> come up with a good way to have this db be more data-like and less
> hardcoded.

I don't have much of an opinion; I was just fixing a build failure.
It does seem nice to have a way to see all the info about the
toolchain without diving into the code.

While it could be argued that these unitialized reads are valid
C99, since the obj type has no padding bits (uint32_t) and C99
doesn't contain some language added in C11 and sebsequent versions,
we can avoid this altogether by moving these warning suppressions
into the loop body.

Also, remove one instance where the loop variable *was* actually
used.
---
Some background:
My motivation for this is to get muon to build with cproc. Currently,
qbe is rather strict on reads of uninitialized variables, so building
muon with cproc fails. While I think this should changed in qbe,
it gets into pretty murky territory in the C standard, so in my
[message trimmed]

I was debugging a build failure in a project, caused by an assertion
failure in qbe:

	Assertion failed: edge[n][0].dead (fold.c: visitjmp: 135)
	Aborted

It turns out that this was due to a bug in the program which did
not initialize some variables, and that has since been fixed.

However, this is different than the usual error you get with used
but not initialized variables ("slot %x is read but never stored
to"). Since it's from an assert, I think there must be some incorrect
assumption here. I haven't investigated any further than this.

Davide Di Paolo <davide.dipaolo09@gmail.com> wrote:
> Hi,
> there is a division by zero bug in decl.c:declarator at line 701.
> 
> 	if (e->u.constant.u > ULLONG_MAX / base.type->size)
> 		error(&tok.loc, "array length is too large");
> 
> The bug is hidden when you compile with gcc because it replaces the 
> division even when no optimization is enabled.
> 
> To reproduce the bug:
> 
> 	$ make CC=clang
> 	$ ./cproc-qbe test/compatible-vla-types.c

If dump_toolchains is passed -t compiler=... without setting the
type of the linker or static_linker, these variables were inspected
prior to being initialized. Initialize them to false so this works
as intended.
---
 src/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main.c b/src/main.c
index 47e047ca..ec47960a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -622,7 +622,7 @@ static bool
 cmd_dump_toolchains(void *_ctx, uint32_t argc, uint32_t argi, char *const argv[])
[message trimmed]

"Sertonix" <sertonix@posteo.net> wrote:
> In 4f206ac1ea (Implement variable length arrays) arrays with the length 0
> were considered to be variable length arrays. This may not be true and
> caused cproc to segfault when parsing for example sizeof(int[0]). Using
> t->prop & PROPVM as check for variable length arrays should always work
> correctly.

Thanks for the patch!

However, the distinction between variably modified type and variably
length array is important here.

Consider this example: