Building upstream GCC with a really new GCC about gccrs HOT 8 OPEN

thanks :) marking this as a good first PR because it's a really silly mistake on my part. the fix is quite easy I think.
this is the offending code:

  tl::optional<NodeId> resolved = tl::nullopt;
  auto label = ctx.labels.get (expr.get_ident ());
  auto value = ctx.values.get (expr.get_ident ());

  if (label)
    resolved = label;
  else if (value)
    resolved = value;
  // TODO: else emit error?

  ctx.map_usage (expr.get_node_id (), *resolved);

which is indeed problematic, because if neither label nor value are valid resolution IDs, then we are accessing a nullopt's value. it's not dangerous or anything, an assertion will pop, but it will indeed be uninitialized I believe.
the correct fix would be to only call map_usage if resolved is a valid value - otherwise, emit an error or at least crash. which hopefully should get rid of the warning

with which function should we look into to emit an error for cause a crash. Can you also point me to the code piece you've shown? Thank you!

this particular piece of code is in gcc/rust/resolve/rust-late-name-resolver-2.0.cc around line 125, in the function void Late::visit (AST::IdentifierExpr &expr)

for emitting an error, you can look at using rust_error_at - there are a lot of examples in the rest of the codebase.

the error emission should look something like this:

rust_error_at(expr.get_locus (), "could not resolve identifier expression: %qs", expr.get_ident ().as_string ().c_str ());

from gccrs.

thanks :) marking this as a good first PR because it's a really silly mistake on my part. the fix is quite easy I think.

this is the offending code:

  tl::optional<NodeId> resolved = tl::nullopt;
  auto label = ctx.labels.get (expr.get_ident ());
  auto value = ctx.values.get (expr.get_ident ());

  if (label)
    resolved = label;
  else if (value)
    resolved = value;
  // TODO: else emit error?

  ctx.map_usage (expr.get_node_id (), *resolved);

which is indeed problematic, because if neither label nor value are valid resolution IDs, then we are accessing a nullopt's value. it's not dangerous or anything, an assertion will pop, but it will indeed be uninitialized I believe.

the correct fix would be to only call map_usage if resolved is a valid value - otherwise, emit an error or at least crash. which hopefully should get rid of the warning

from gccrs.

thanks :) marking this as a good first PR because it's a really silly mistake on my part. the fix is quite easy I think.

this is the offending code:

  tl::optional<NodeId> resolved = tl::nullopt;
  auto label = ctx.labels.get (expr.get_ident ());
  auto value = ctx.values.get (expr.get_ident ());

  if (label)
    resolved = label;
  else if (value)
    resolved = value;
  // TODO: else emit error?

  ctx.map_usage (expr.get_node_id (), *resolved);

which is indeed problematic, because if neither label nor value are valid resolution IDs, then we are accessing a nullopt's value. it's not dangerous or anything, an assertion will pop, but it will indeed be uninitialized I believe.

the correct fix would be to only call map_usage if resolved is a valid value - otherwise, emit an error or at least crash. which hopefully should get rid of the warning

with which function should we look into to emit an error for cause a crash. Can you also point me to the code piece you've shown? Thank you!

from gccrs.

should I assign you the issue @badumbatish? I think it will require building g++ from the commit @jbglaw mentioned and using this to build gccrs in order to ensure it's resolved

from gccrs.

Just needs to be a "fairly recent" GCC, from the past few days. I'd also offer to test patches by running them through my CI pipeline with --enable-werror-always.

from gccrs.

should I assign you the issue @badumbatish? I think it will require building g++ from the commit @jbglaw mentioned and using this to build gccrs in order to ensure it's resolved

oops i didn't see this. Yes i'll try on this issue

from gccrs.

thanks :) marking this as a good first PR because it's a really silly mistake on my part. the fix is quite easy I think.
this is the offending code:

  tl::optional<NodeId> resolved = tl::nullopt;
  auto label = ctx.labels.get (expr.get_ident ());
  auto value = ctx.values.get (expr.get_ident ());

  if (label)
    resolved = label;
  else if (value)
    resolved = value;
  // TODO: else emit error?

  ctx.map_usage (expr.get_node_id (), *resolved);

which is indeed problematic, because if neither label nor value are valid resolution IDs, then we are accessing a nullopt's value. it's not dangerous or anything, an assertion will pop, but it will indeed be uninitialized I believe.
the correct fix would be to only call map_usage if resolved is a valid value - otherwise, emit an error or at least crash. which hopefully should get rid of the warning

with which function should we look into to emit an error for cause a crash. Can you also point me to the code piece you've shown? Thank you!

this particular piece of code is in gcc/rust/resolve/rust-late-name-resolver-2.0.cc around line 125, in the function void Late::visit (AST::IdentifierExpr &expr)

for emitting an error, you can look at using rust_error_at - there are a lot of examples in the rest of the codebase.

the error emission should look something like this:

rust_error_at(expr.get_locus (), "could not resolve identifier expression: %qs", expr.get_ident ().as_string ().c_str ());

I just realized the make error output shows the file crash location ....

from gccrs.