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

Implement basic bool and int formatting for diagnostics #4411

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions toolchain/check/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ cc_library(
"//toolchain/check:generic_region_stack",
"//toolchain/check:scope_stack",
"//toolchain/diagnostics:diagnostic_emitter",
"//toolchain/diagnostics:format_providers",
"//toolchain/lex:token_kind",
"//toolchain/lex:tokenized_buffer",
"//toolchain/parse:node_kind",
Expand Down Expand Up @@ -116,6 +117,7 @@ cc_library(
"//toolchain/base:pretty_stack_trace_function",
"//toolchain/base:value_store",
"//toolchain/diagnostics:diagnostic_emitter",
"//toolchain/diagnostics:format_providers",
"//toolchain/lex:token_index",
"//toolchain/lex:token_kind",
"//toolchain/lex:tokenized_buffer",
Expand Down
42 changes: 23 additions & 19 deletions toolchain/check/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "toolchain/check/context.h"
#include "toolchain/check/operator.h"
#include "toolchain/check/pattern_match.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/sem_ir/copy_on_write_block.h"
#include "toolchain/sem_ir/file.h"
#include "toolchain/sem_ir/generic.h"
Expand Down Expand Up @@ -381,8 +382,13 @@ static auto ConvertStructToStructOrClass(Context& context,
SemIR::StructType src_type,
SemIR::StructType dest_type,
SemIR::InstId value_id,
ConversionTarget target, bool is_class)
ConversionTarget target)
-> SemIR::InstId {
static_assert(std::is_same_v<SemIR::ClassElementAccess, TargetAccessInstT> ||
std::is_same_v<SemIR::StructAccess, TargetAccessInstT>);
constexpr bool ToClass =
std::is_same_v<SemIR::ClassElementAccess, TargetAccessInstT>;

auto& sem_ir = context.sem_ir();
auto src_elem_fields = sem_ir.inst_blocks().Get(src_type.fields_id);
auto dest_elem_fields = sem_ir.inst_blocks().Get(dest_type.fields_id);
Expand All @@ -406,14 +412,14 @@ static auto ConvertStructToStructOrClass(Context& context,
// TODO: If not, include the name of the first source field that doesn't
// exist in the destination or vice versa in the diagnostic.
if (src_elem_fields.size() != dest_elem_fields.size()) {
CARBON_DIAGNOSTIC(StructInitElementCountMismatch, Error,
"cannot initialize {0} with {1} field(s) from struct "
"with {2} field(s).",
llvm::StringLiteral, size_t, size_t);
context.emitter().Emit(
value_loc_id, StructInitElementCountMismatch,
is_class ? llvm::StringLiteral("class") : llvm::StringLiteral("struct"),
dest_elem_fields.size(), src_elem_fields.size());
CARBON_DIAGNOSTIC(
StructInitElementCountMismatch, Error,
"cannot initialize {0:class|struct} with {1} field(s) from struct "
"with {2} field(s).",
BoolAsSelect, size_t, size_t);
context.emitter().Emit(value_loc_id, StructInitElementCountMismatch,
ToClass, dest_elem_fields.size(),
src_elem_fields.size());
return SemIR::InstId::BuiltinError;
}

Expand Down Expand Up @@ -493,7 +499,7 @@ static auto ConvertStructToStructOrClass(Context& context,
new_block.Set(i, init_id);
}

if (is_class) {
if (ToClass) {
target.init_block->InsertHere();
CARBON_CHECK(is_init,
"Converting directly to a class value is not supported");
Expand Down Expand Up @@ -522,7 +528,7 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type,
SemIR::InstId value_id,
ConversionTarget target) -> SemIR::InstId {
return ConvertStructToStructOrClass<SemIR::StructAccess>(
context, src_type, dest_type, value_id, target, /*is_class=*/false);
context, src_type, dest_type, value_id, target);
}

// Performs a conversion from a struct to a class type. This function only
Expand Down Expand Up @@ -554,7 +560,7 @@ static auto ConvertStructToClass(Context& context, SemIR::StructType src_type,
}

auto result_id = ConvertStructToStructOrClass<SemIR::ClassElementAccess>(
context, src_type, dest_struct_type, value_id, target, /*is_class=*/true);
context, src_type, dest_struct_type, value_id, target);

if (need_temporary) {
target_block.InsertHere();
Expand Down Expand Up @@ -1154,13 +1160,11 @@ static auto ConvertSelf(Context& context, SemIR::LocId call_loc_id,
bool addr_pattern = context.insts().Is<SemIR::AddrPattern>(self_param_id);
DiagnosticAnnotationScope annotate_diagnostics(
&context.emitter(), [&](auto& builder) {
CARBON_DIAGNOSTIC(
InCallToFunctionSelf, Note,
"initializing `{0}` parameter of method declared here",
llvm::StringLiteral);
builder.Note(self_param_id, InCallToFunctionSelf,
addr_pattern ? llvm::StringLiteral("addr self")
: llvm::StringLiteral("self"));
CARBON_DIAGNOSTIC(InCallToFunctionSelf, Note,
"initializing `{0:addr self|self}` parameter of "
"method declared here",
BoolAsSelect);
builder.Note(self_param_id, InCallToFunctionSelf, addr_pattern);
});

return CallerPatternMatch(context, callee_specific_id, self_param_id,
Expand Down
20 changes: 10 additions & 10 deletions toolchain/check/eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "toolchain/check/diagnostic_helpers.h"
#include "toolchain/check/generic.h"
#include "toolchain/diagnostics/diagnostic_emitter.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/sem_ir/builtin_function_kind.h"
#include "toolchain/sem_ir/function.h"
#include "toolchain/sem_ir/generic.h"
Expand Down Expand Up @@ -745,18 +746,17 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc,
// Bit shift.
case SemIR::BuiltinFunctionKind::IntLeftShift:
case SemIR::BuiltinFunctionKind::IntRightShift:
op_str = (builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift)
? llvm::StringLiteral("<<")
: llvm::StringLiteral(">>");
if (rhs_val.uge(lhs_val.getBitWidth()) ||
(rhs_val.isNegative() && context.types().IsSignedInt(rhs.type_id))) {
CARBON_DIAGNOSTIC(CompileTimeShiftOutOfRange, Error,
"shift distance not in range [0, {0}) in {1} {2} {3}",
unsigned, TypedInt, llvm::StringLiteral, TypedInt);
context.emitter().Emit(loc, CompileTimeShiftOutOfRange,
lhs_val.getBitWidth(),
{.type = lhs.type_id, .value = lhs_val}, op_str,
{.type = rhs.type_id, .value = rhs_val});
CARBON_DIAGNOSTIC(
CompileTimeShiftOutOfRange, Error,
"shift distance not in range [0, {0}) in {1} {2:<<|>>} {3}",
unsigned, TypedInt, BoolAsSelect, TypedInt);
context.emitter().Emit(
loc, CompileTimeShiftOutOfRange, lhs_val.getBitWidth(),
{.type = lhs.type_id, .value = lhs_val},
builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift,
{.type = rhs.type_id, .value = rhs_val});
// TODO: Is it useful to recover by returning 0 or -1?
return SemIR::ConstantId::Error;
}
Expand Down
25 changes: 11 additions & 14 deletions toolchain/check/handle_binding_pattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "toolchain/check/convert.h"
#include "toolchain/check/handle.h"
#include "toolchain/check/return.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/sem_ir/ids.h"
#include "toolchain/sem_ir/inst.h"

Expand Down Expand Up @@ -104,23 +105,19 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
cast_type_id,
[&] {
CARBON_DIAGNOSTIC(IncompleteTypeInVarDecl, Error,
"{0} has incomplete type {1}",
llvm::StringLiteral, SemIR::TypeId);
return context.emitter().Build(
type_node, IncompleteTypeInVarDecl,
parent_class_decl ? llvm::StringLiteral("field")
: llvm::StringLiteral("variable"),
cast_type_id);
"{0:field|variable} has incomplete type {1}",
BoolAsSelect, SemIR::TypeId);
return context.emitter().Build(type_node, IncompleteTypeInVarDecl,
parent_class_decl.has_value(),
cast_type_id);
},
[&] {
CARBON_DIAGNOSTIC(AbstractTypeInVarDecl, Error,
"{0} has abstract type {1}", llvm::StringLiteral,
SemIR::TypeId);
return context.emitter().Build(
type_node, AbstractTypeInVarDecl,
parent_class_decl ? llvm::StringLiteral("field")
: llvm::StringLiteral("variable"),
cast_type_id);
"{0:field|variable} has abstract type {1}",
BoolAsSelect, SemIR::TypeId);
return context.emitter().Build(type_node, AbstractTypeInVarDecl,
parent_class_decl.has_value(),
cast_type_id);
});
if (parent_class_decl) {
CARBON_CHECK(context_node_kind == Parse::NodeKind::VariableIntroducer,
Expand Down
78 changes: 42 additions & 36 deletions toolchain/check/merge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "toolchain/base/kind_switch.h"
#include "toolchain/check/import_ref.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/sem_ir/ids.h"
#include "toolchain/sem_ir/typed_insts.h"

Expand Down Expand Up @@ -193,10 +194,12 @@ static auto EntityHasParamError(Context& context, const DeclParams& info)

// Returns false if a param differs for a redeclaration. The caller is expected
// to provide a diagnostic.
static auto CheckRedeclParam(
Context& context, llvm::StringLiteral param_diag_label, int32_t param_index,
SemIR::InstId new_param_pattern_id, SemIR::InstId prev_param_pattern_id,
SemIR::SpecificId prev_specific_id, bool diagnose) -> bool {
static auto CheckRedeclParam(Context& context, bool is_implicit_param,
int32_t param_index,
SemIR::InstId new_param_pattern_id,
SemIR::InstId prev_param_pattern_id,
SemIR::SpecificId prev_specific_id, bool diagnose)
-> bool {
// TODO: Consider differentiating between type and name mistakes. For now,
// taking the simpler approach because I also think we may want to refactor
// params.
Expand All @@ -205,15 +208,16 @@ static auto CheckRedeclParam(
return;
}
CARBON_DIAGNOSTIC(RedeclParamDiffers, Error,
"redeclaration differs at {0}parameter {1}",
llvm::StringLiteral, int32_t);
CARBON_DIAGNOSTIC(RedeclParamPrevious, Note,
"previous declaration's corresponding {0}parameter here",
llvm::StringLiteral);
"redeclaration differs at {0:implicit |}parameter {1}",
BoolAsSelect, int32_t);
CARBON_DIAGNOSTIC(
RedeclParamPrevious, Note,
"previous declaration's corresponding {0:implicit |}parameter here",
BoolAsSelect);
context.emitter()
.Build(new_param_pattern_id, RedeclParamDiffers, param_diag_label,
.Build(new_param_pattern_id, RedeclParamDiffers, is_implicit_param,
param_index + 1)
.Note(prev_param_pattern_id, RedeclParamPrevious, param_diag_label)
.Note(prev_param_pattern_id, RedeclParamPrevious, is_implicit_param)
.Emit();
};

Expand Down Expand Up @@ -265,7 +269,7 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
SemIR::InstBlockId new_param_patterns_id,
SemIRLoc prev_decl_loc,
SemIR::InstBlockId prev_param_patterns_id,
llvm::StringLiteral param_diag_label,
bool is_implicit_param,
SemIR::SpecificId prev_specific_id, bool diagnose)
-> bool {
// This will often occur for empty params.
Expand All @@ -279,18 +283,18 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
return false;
}
CARBON_DIAGNOSTIC(RedeclParamListDiffers, Error,
"redeclaration differs because of {1}{0}parameter list",
llvm::StringLiteral, llvm::StringLiteral);
"redeclaration differs because of "
"{1:'|missing '}{0:implicit |}parameter list",
BoolAsSelect, BoolAsSelect);
CARBON_DIAGNOSTIC(RedeclParamListPrevious, Note,
"previously declared with{1} {0}parameter list",
llvm::StringLiteral, llvm::StringLiteral);
"previously declared "
"{1:with|without} {0:implicit |}parameter list",
BoolAsSelect, BoolAsSelect);
context.emitter()
.Build(new_decl_loc, RedeclParamListDiffers, param_diag_label,
new_param_patterns_id.is_valid() ? llvm::StringLiteral("")
: "missing ")
.Note(
prev_decl_loc, RedeclParamListPrevious, param_diag_label,
prev_param_patterns_id.is_valid() ? llvm::StringLiteral("") : "out")
.Build(new_decl_loc, RedeclParamListDiffers, is_implicit_param,
new_param_patterns_id.is_valid())
.Note(prev_decl_loc, RedeclParamListPrevious, is_implicit_param,
prev_param_patterns_id.is_valid())
.Emit();
return false;
}
Expand All @@ -307,22 +311,23 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
}
CARBON_DIAGNOSTIC(
RedeclParamCountDiffers, Error,
"redeclaration differs because of {0}parameter count of {1}",
llvm::StringLiteral, int32_t);
CARBON_DIAGNOSTIC(RedeclParamCountPrevious, Note,
"previously declared with {0}parameter count of {1}",
llvm::StringLiteral, int32_t);
"redeclaration differs because of {0:implicit |}parameter count of {1}",
BoolAsSelect, int32_t);
CARBON_DIAGNOSTIC(
RedeclParamCountPrevious, Note,
"previously declared with {0:implicit |}parameter count of {1}",
BoolAsSelect, int32_t);
context.emitter()
.Build(new_decl_loc, RedeclParamCountDiffers, param_diag_label,
.Build(new_decl_loc, RedeclParamCountDiffers, is_implicit_param,
new_param_pattern_ids.size())
.Note(prev_decl_loc, RedeclParamCountPrevious, param_diag_label,
.Note(prev_decl_loc, RedeclParamCountPrevious, is_implicit_param,
prev_param_pattern_ids.size())
.Emit();
return false;
}
for (auto [index, new_param_pattern_id, prev_param_pattern_id] :
llvm::enumerate(new_param_pattern_ids, prev_param_pattern_ids)) {
if (!CheckRedeclParam(context, param_diag_label, index,
if (!CheckRedeclParam(context, is_implicit_param, index,
new_param_pattern_id, prev_param_pattern_id,
prev_specific_id, diagnose)) {
return false;
Expand Down Expand Up @@ -410,15 +415,16 @@ auto CheckRedeclParamsMatch(Context& context, const DeclParams& new_entity,
EntityHasParamError(context, prev_entity)) {
return false;
}
if (!CheckRedeclParams(context, new_entity.loc,
new_entity.implicit_param_patterns_id, prev_entity.loc,
prev_entity.implicit_param_patterns_id, "implicit ",
prev_specific_id, diagnose)) {
if (!CheckRedeclParams(
context, new_entity.loc, new_entity.implicit_param_patterns_id,
prev_entity.loc, prev_entity.implicit_param_patterns_id,
/*is_implicit_param=*/true, prev_specific_id, diagnose)) {
return false;
}
if (!CheckRedeclParams(context, new_entity.loc, new_entity.param_patterns_id,
prev_entity.loc, prev_entity.param_patterns_id, "",
prev_specific_id, diagnose)) {
prev_entity.loc, prev_entity.param_patterns_id,
/*is_implicit_param=*/false, prev_specific_id,
diagnose)) {
return false;
}
if (check_syntax &&
Expand Down
26 changes: 26 additions & 0 deletions toolchain/diagnostics/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ cc_library(
],
deps = [
":diagnostic_kind",
":format_providers",
"//common:check",
"//common:ostream",
"@llvm-project//llvm:Support",
Expand Down Expand Up @@ -56,6 +57,31 @@ cc_library(
],
)

cc_library(
name = "format_providers",
srcs = ["format_providers.cpp"],
hdrs = ["format_providers.h"],
deps = [
"//common:check",
"//common:ostream",
"@llvm-project//llvm:Support",
],
)

cc_test(
name = "format_providers_test",
size = "small",
srcs = ["format_providers_test.cpp"],
deps = [
":diagnostic_emitter",
":format_providers",
":mocks",
"//testing/base:gtest_main",
"@googletest//:gtest",
"@llvm-project//llvm:Support",
],
)

cc_library(
name = "null_diagnostics",
hdrs = ["null_diagnostics.h"],
Expand Down
Loading
Loading