carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-10-05 11:11:05 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Clean up format_provider uses (#4417)

Browse Source 
Building on https://github.com/carbon-language/carbon-lang/pull/4411,
replace format_provider uses (other than `TokenKind`, which is more on
the okay side of things)

Also does some edits to `ClassMemberDefinition` to try to better match
diagnostic style
This commit is contained in:
Jon Ross-Perkins
2024-10-17 18:47:29 +00:00
committed by GitHub
parent a02dfe0226
commit 5bdeb010c8
8 changed files with 39 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/context.cpp
+9 -10
View File
@@ -21,6 +21,7 @@
#include "toolchain/check/inst_block_stack.h"
#include "toolchain/check/merge.h"
#include "toolchain/diagnostics/diagnostic_emitter.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/lex/tokenized_buffer.h"
#include "toolchain/parse/node_ids.h"
#include "toolchain/parse/node_kind.h"
@@ -362,13 +363,6 @@ static auto DiagnoseInvalidQualifiedNameAccess(Context& context, SemIRLoc loc,
// TODO: Support scoped entities other than just classes.
auto class_info = context.classes().Get(class_type->class_id);
CARBON_DIAGNOSTIC(ClassInvalidMemberAccess, Error,
"cannot access {0} member `{1}` of type {2}",
SemIR::AccessKind, SemIR::NameId, SemIR::TypeId);
CARBON_DIAGNOSTIC(ClassMemberDefinition, Note,
"the {0} member `{1}` is defined here", SemIR::AccessKind,
SemIR::NameId);
auto parent_type_id = class_info.self_type_id;
if (access_kind == SemIR::AccessKind::Private && is_parent_access) {
@@ -384,10 +378,15 @@ static auto DiagnoseInvalidQualifiedNameAccess(Context& context, SemIRLoc loc,
}
}
CARBON_DIAGNOSTIC(
ClassInvalidMemberAccess, Error,
"cannot access {0:private|protected} member `{1}` of type {2}",
BoolAsSelect, SemIR::NameId, SemIR::TypeId);
CARBON_DIAGNOSTIC(ClassMemberDeclaration, Note, "declared here");
context.emitter()
.Build(loc, ClassInvalidMemberAccess, access_kind, name_id,
parent_type_id)
.Note(scope_result_id, ClassMemberDefinition, access_kind, name_id)
.Build(loc, ClassInvalidMemberAccess,
access_kind == SemIR::AccessKind::Private, name_id, parent_type_id)
.Note(scope_result_id, ClassMemberDeclaration)
.Emit();
}
toolchain/check/testdata/class/access_modifers.carbon
Vendored
+7 -7
View File
@@ -30,7 +30,7 @@ fn Run() {
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE+7]]:21: error: cannot access private member `radius` of type `Circle`
// CHECK:STDERR: let radius: i32 = circle.radius;
// CHECK:STDERR: ^~~~~~~~~~~~~
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-17]]:15: note: the private member `radius` is defined here
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-17]]:15: note: declared here
// CHECK:STDERR: private var radius: i32;
// CHECK:STDERR: ^~~~~~~~~~~
// CHECK:STDERR:
@@ -38,7 +38,7 @@ fn Run() {
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE+7]]:3: error: cannot access private member `radius` of type `Circle`
// CHECK:STDERR: circle.radius = 5;
// CHECK:STDERR: ^~~~~~~~~~~~~
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-25]]:15: note: the private member `radius` is defined here
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-25]]:15: note: declared here
// CHECK:STDERR: private var radius: i32;
// CHECK:STDERR: ^~~~~~~~~~~
// CHECK:STDERR:
@@ -46,7 +46,7 @@ fn Run() {
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE+7]]:3: error: cannot access private member `SOME_INTERNAL_CONSTANT` of type `Circle`
// CHECK:STDERR: circle.SOME_INTERNAL_CONSTANT;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-32]]:15: note: the private member `SOME_INTERNAL_CONSTANT` is defined here
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-32]]:15: note: declared here
// CHECK:STDERR: private let SOME_INTERNAL_CONSTANT: i32 = 5;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
@@ -55,7 +55,7 @@ fn Run() {
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE+7]]:3: error: cannot access private member `SomeInternalFunction` of type `Circle`
// CHECK:STDERR: circle.SomeInternalFunction();
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-39]]:3: note: the private member `SomeInternalFunction` is defined here
// CHECK:STDERR: fail_private_field_access.carbon:[[@LINE-39]]:3: note: declared here
// CHECK:STDERR: private fn SomeInternalFunction() -> i32 {
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
@@ -74,7 +74,7 @@ fn Run() {
// CHECK:STDERR: fail_protected_field_access.carbon:[[@LINE+7]]:16: error: cannot access protected member `x` of type `A`
// CHECK:STDERR: let x: i32 = A.x;
// CHECK:STDERR: ^~~
// CHECK:STDERR: fail_protected_field_access.carbon:[[@LINE-7]]:17: note: the protected member `x` is defined here
// CHECK:STDERR: fail_protected_field_access.carbon:[[@LINE-7]]:17: note: declared here
// CHECK:STDERR: protected var x: i32;
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR:
@@ -123,7 +123,7 @@ class A {
// CHECK:STDERR: fail_global_access.carbon:[[@LINE+7]]:14: error: cannot access protected member `x` of type `A`
// CHECK:STDERR: let x: i32 = A.x;
// CHECK:STDERR: ^~~
// CHECK:STDERR: fail_global_access.carbon:[[@LINE-7]]:17: note: the protected member `x` is defined here
// CHECK:STDERR: fail_global_access.carbon:[[@LINE-7]]:17: note: declared here
// CHECK:STDERR: protected let x: i32 = 5;
// CHECK:STDERR: ^
// CHECK:STDERR:
@@ -131,7 +131,7 @@ let x: i32 = A.x;
// CHECK:STDERR: fail_global_access.carbon:[[@LINE+6]]:14: error: cannot access private member `y` of type `A`
// CHECK:STDERR: let y: i32 = A.y;
// CHECK:STDERR: ^~~
// CHECK:STDERR: fail_global_access.carbon:[[@LINE-14]]:15: note: the private member `y` is defined here
// CHECK:STDERR: fail_global_access.carbon:[[@LINE-14]]:15: note: declared here
// CHECK:STDERR: private let y: i32 = 5;
// CHECK:STDERR: ^
let y: i32 = A.y;
toolchain/check/testdata/class/inheritance_access.carbon
Vendored
+6 -6
View File
@@ -82,7 +82,7 @@ class Square {
// CHECK:STDERR: fail_inherited_private_field_access.carbon:[[@LINE+7]]:12: error: cannot access private member `y` of type `Shape`
// CHECK:STDERR: return self.y;
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: fail_inherited_private_field_access.carbon:[[@LINE-10]]:15: note: the private member `y` is defined here
// CHECK:STDERR: fail_inherited_private_field_access.carbon:[[@LINE-10]]:15: note: declared here
// CHECK:STDERR: private var y: i32;
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR:
@@ -121,7 +121,7 @@ class C {
// CHECK:STDERR: fail_noninstance_private_on_parent.carbon:[[@LINE+7]]:12: error: cannot access private member `F` of type `B`
// CHECK:STDERR: fn G() { Self.F(); }
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR: fail_noninstance_private_on_parent.carbon:[[@LINE-8]]:3: note: the private member `F` is defined here
// CHECK:STDERR: fail_noninstance_private_on_parent.carbon:[[@LINE-8]]:3: note: declared here
// CHECK:STDERR: private fn F() {}
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
// CHECK:STDERR:
@@ -161,7 +161,7 @@ class B {
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE+7]]:5: error: cannot access private member `SOME_PRIVATE_CONSTANT` of type `A`
// CHECK:STDERR: A.SOME_PRIVATE_CONSTANT;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE-14]]:15: note: the private member `SOME_PRIVATE_CONSTANT` is defined here
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE-14]]:15: note: declared here
// CHECK:STDERR: private let SOME_PRIVATE_CONSTANT: i32 = 5;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
@@ -170,7 +170,7 @@ class B {
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE+7]]:12: error: cannot access protected member `SOME_PROTECTED_CONSTANT` of type `A`
// CHECK:STDERR: return A.SOME_PROTECTED_CONSTANT;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE-24]]:17: note: the protected member `SOME_PROTECTED_CONSTANT` is defined here
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE-24]]:17: note: declared here
// CHECK:STDERR: protected let SOME_PROTECTED_CONSTANT: i32 = 5;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
@@ -181,7 +181,7 @@ class B {
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE+7]]:12: error: cannot access protected member `INTERNAL_CONSTANT` of type `Internal`
// CHECK:STDERR: return self.internal.INTERNAL_CONSTANT;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE-30]]:17: note: the protected member `INTERNAL_CONSTANT` is defined here
// CHECK:STDERR: fail_non_inherited_access.carbon:[[@LINE-30]]:17: note: declared here
// CHECK:STDERR: protected let INTERNAL_CONSTANT: i32 = 5;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~
// CHECK:STDERR:
@@ -203,7 +203,7 @@ class B {
// CHECK:STDERR: fail_compound_member_access.carbon:[[@LINE+6]]:11: error: cannot access private member `x` of type `A`
// CHECK:STDERR: self.(A.x);
// CHECK:STDERR: ^~~
// CHECK:STDERR: fail_compound_member_access.carbon:[[@LINE-9]]:15: note: the private member `x` is defined here
// CHECK:STDERR: fail_compound_member_access.carbon:[[@LINE-9]]:15: note: declared here
// CHECK:STDERR: private var x: i32;
// CHECK:STDERR: ^~~~~~
self.(A.x);
toolchain/diagnostics/diagnostic_kind.def
+1 -1
View File
@@ -367,7 +367,7 @@ CARBON_DIAGNOSTIC_KIND(ExternLibraryOnDefinition)
CARBON_DIAGNOSTIC_KIND(ExternLibraryIsCurrentLibrary)
// Access modifiers.
CARBON_DIAGNOSTIC_KIND(ClassMemberDefinition)
CARBON_DIAGNOSTIC_KIND(ClassMemberDeclaration)
CARBON_DIAGNOSTIC_KIND(ClassInvalidMemberAccess)
// Alias diagnostics.
toolchain/docs/diagnostics.md
+8 -4
View File
@@ -171,10 +171,14 @@ methods for formatting arguments:
allocations are required.
- `llvm::StringRef` is disallowed due to lifetime issues.
- `llvm::format_provider<...>` specializations.
- This can be used when formatting the parameter doesn't require
additional context.
- For example, `Lex::TokenKind` and `Parse::RelativeLoc` provide
diagnostic formatting this way.
- `BoolAsSelect` and `IntAsSelect` from
[format_providers.h](/toolchain/diagnostics/format_providers.h) are
recommended for many cases, because they allow putting the output string
in the format.
- `IntAsSelect` can also be used to support pluralization.
- Custom providers can also be added for non-translated values. For
example, `Lex::TokenKind` refers to syntax elements, and so can safely
have its own format provider.
- `DiagnosticConverter::ConvertArg` overrides.
- This can provide additional context to a formatter.
- For example, formatting `SemIR::NameId` accesses the IR's name list.
toolchain/lex/BUILD
+1
View File
@@ -74,6 +74,7 @@ cc_library(
":helpers",
"//common:check",
"//toolchain/diagnostics:diagnostic_emitter",
"//toolchain/diagnostics:format_providers",
"@llvm-project//llvm:Support",
],
)
toolchain/lex/numeric_literal.cpp
+7 -25
View File
@@ -9,30 +9,10 @@
#include "common/check.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/FormatVariadicDetails.h"
#include "toolchain/diagnostics/format_providers.h"
#include "toolchain/lex/character_set.h"
#include "toolchain/lex/helpers.h"
// We use formatv primarily for diagnostics. In these cases, it's expected that
// the spelling in source code should be used.
template <>
struct llvm::format_provider<Carbon::Lex::NumericLiteral::Radix> {
using Radix = Carbon::Lex::NumericLiteral::Radix;
static void format(const Radix& radix, raw_ostream& out,
StringRef /*style*/) {
switch (radix) {
case Radix::Binary:
out << "binary";
break;
case Radix::Decimal:
out << "decimal";
break;
case Radix::Hexadecimal:
out << "hexadecimal";
break;
}
}
};
namespace Carbon::Lex {
auto NumericLiteral::Lex(llvm::StringRef source_text)
@@ -308,10 +288,12 @@ auto NumericLiteral::Parser::CheckDigitSequence(llvm::StringRef text,
continue;
}
CARBON_DIAGNOSTIC(InvalidDigit, Error,
"invalid digit '{0}' in {1} numeric literal", char,
NumericLiteral::Radix);
emitter_.Emit(text.begin() + i, InvalidDigit, c, radix);
CARBON_DIAGNOSTIC(
InvalidDigit, Error,
"invalid digit '{0}' in {1:=2:binary|=10:decimal|=16:hexadecimal} "
"numeric literal",
char, IntAsSelect);
emitter_.Emit(text.begin() + i, InvalidDigit, c, static_cast<int>(radix));
return {.ok = false};
}
toolchain/sem_ir/name_scope.h
-23
View File
@@ -18,29 +18,6 @@ enum class AccessKind : int8_t {
Private,
};
} // namespace Carbon::SemIR
template <>
struct llvm::format_provider<Carbon::SemIR::AccessKind> {
using AccessKind = Carbon::SemIR::AccessKind;
static void format(const AccessKind& loc, raw_ostream& out,
StringRef /*style*/) {
switch (loc) {
case AccessKind::Private:
out << "private";
break;
case AccessKind::Protected:
out << "protected";
break;
case AccessKind::Public:
out << "public";
break;
}
}
};
namespace Carbon::SemIR {
struct NameScope : Printable<NameScope> {
struct Entry {
NameId name_id;