Skip to content

Commit ebb127f

Browse files
bors[bot]davidmalcolmCohenArthur
authored
1408: Experiment: add optional error codes to diagnostics r=CohenArthur a=davidmalcolm rustc has error codes to identify specific errors, with URLs documenting each error. In GCC 13 I've extended GCC's diagnostic subsystem so that a diagnostic can be associated with zero or more `diagnostic_metadata::rule` instances, which have a textual description and a URL. I meant this for rules in coding standards and specifications, but it struck me that potentially gccrs could reuse the same error codes as rustc. The following pull request implements an experimental form of this; I picked a single error at random: [E0054](https://doc.rust-lang.org/error-index.html#E0054). With this patch, gccrs emits e.g.: ``` bad_as_bool_char.rs:8:19: error: invalid cast [u8] to [bool] [E0054] 8 | let nb = 0u8 as bool; | ~ ^ ``` where the trailing [E0054] is colorized, and, in a suitably capable terminal is a clickable URL to https://doc.rust-lang.org/error-index.html#E0054. The error code is after the diagnostic message, whereas rustc puts it after the word "error". I could change that in gcc's diagnostic.cc, perhaps. I'm not sure if this is a good idea (e.g. is it OK and maintainable to share error codes with rustc?), but thought it was worth sharing. 1503: Add overflow traps r=CohenArthur a=CohenArthur Opening as a draft since the code is really ugly and some tests still fail. I am looking for feedback on the implementation and my approximative use of functions like `temporary_variable` which I feel I might be using the wrong way. I'll open up an issue of what still needs to be done, namely: - [x] Properly handle sub and mul operations - [ ] Disable the check in `release` mode (`-frust-disable-overflow-checks`) - [ ] Handle specific rust overflow attribute (needs checkup on the name) I'll open up some PRs to clean up some parts of the backend as well. 1511: lint: Do not emit unused warnings for public items r=CohenArthur a=CohenArthur This fixes the overzealous warnings on public items from the unused pass Co-authored-by: David Malcolm <[email protected]> Co-authored-by: Arthur Cohen <[email protected]>
4 parents 45f80a2 + 2a60741 + fc82b68 + 702bb4b commit ebb127f

25 files changed

+420
-99
lines changed

.github/bors_log_expected_warnings

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
../../gcc/config/i386/i386.cc:2565:8: warning: too many arguments for format [-Wformat-extra-args]
8585
../../gcc/config/i386/i386.cc:2565:8: warning: unknown conversion type character ‘{’ in format [-Wformat=]
8686
../../gcc/config/i386/i386.cc:2565:8: warning: unknown conversion type character ‘}’ in format [-Wformat=]
87-
../../gcc/diagnostic.cc:2191:52: warning: format not a string literal and no format arguments [-Wformat-security]
87+
../../gcc/diagnostic.cc:2206:52: warning: format not a string literal and no format arguments [-Wformat-security]
8888
../../gcc/doc/sourcebuild.texi:1452: warning: node `Add Options' is next for `Effective-Target Keywords' in menu but not in sectioning
8989
../../gcc/doc/sourcebuild.texi:2946: warning: node `Effective-Target Keywords' is prev for `Add Options' in menu but not in sectioning
9090
../../gcc/fold-const.cc:314:42: warning: format not a string literal and no format arguments [-Wformat-security]

gcc/diagnostic-core.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ extern void error_n (location_t, unsigned HOST_WIDE_INT, const char *,
9292
extern void error_at (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
9393
extern void error_at (rich_location *, const char *, ...)
9494
ATTRIBUTE_GCC_DIAG(2,3);
95+
extern void error_meta (rich_location *, const diagnostic_metadata &,
96+
const char *, ...)
97+
ATTRIBUTE_GCC_DIAG(3,4);
9598
extern void fatal_error (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3)
9699
ATTRIBUTE_NORETURN;
97100
/* Pass one of the OPT_W* from options.h as the second parameter. */

gcc/diagnostic.cc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2050,6 +2050,21 @@ error_at (rich_location *richloc, const char *gmsgid, ...)
20502050
va_end (ap);
20512051
}
20522052

2053+
/* Same as above, but with metadata. */
2054+
2055+
void
2056+
error_meta (rich_location *richloc, const diagnostic_metadata &metadata,
2057+
const char *gmsgid, ...)
2058+
{
2059+
gcc_assert (richloc);
2060+
2061+
auto_diagnostic_group d;
2062+
va_list ap;
2063+
va_start (ap, gmsgid);
2064+
diagnostic_impl (richloc, &metadata, -1, gmsgid, &ap, DK_ERROR);
2065+
va_end (ap);
2066+
}
2067+
20532068
/* "Sorry, not implemented." Use for a language feature which is
20542069
required by the relevant specification but not implemented by GCC.
20552070
An object file will not be produced. */

gcc/rust/backend/rust-builtins.h

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@
1818
#define RUST_BUILTINS_H
1919

2020
#include "rust-system.h"
21-
#include "tree.h"
21+
#include "rust-tree.h"
2222
#include "langhooks.h"
23+
#include "tree.h"
2324

2425
namespace Rust {
2526
namespace Compile {
@@ -99,16 +100,34 @@ class BuiltinsContext
99100

100101
BuiltinsContext () { setup (); }
101102

102-
void setup ()
103+
void setup_overflow_fns ()
104+
{
105+
tree overflow_type
106+
= build_varargs_function_type_list (boolean_type_node, NULL_TREE);
107+
108+
define_builtin ("add_overflow", BUILT_IN_ADD_OVERFLOW,
109+
"__builtin_add_overflow", "add_overflow", overflow_type, 0);
110+
define_builtin ("sub_overflow", BUILT_IN_SUB_OVERFLOW,
111+
"__builtin_sub_overflow", "sub_overflow", overflow_type, 0);
112+
define_builtin ("mul_overflow", BUILT_IN_MUL_OVERFLOW,
113+
"__builtin_mul_overflow", "mul_overflow", overflow_type, 0);
114+
}
115+
116+
void setup_math_fns ()
103117
{
104118
tree math_function_type_f32
105119
= build_function_type_list (float_type_node, float_type_node, NULL_TREE);
106120

107121
define_builtin ("sinf32", BUILT_IN_SINF, "__builtin_sinf", "sinf",
108122
math_function_type_f32, builtin_const);
109-
110123
define_builtin ("sqrtf32", BUILT_IN_SQRTF, "__builtin_sqrtf", "sqrtf",
111124
math_function_type_f32, builtin_const);
125+
}
126+
127+
void setup ()
128+
{
129+
setup_math_fns ();
130+
setup_overflow_fns ();
112131

113132
define_builtin ("unreachable", BUILT_IN_UNREACHABLE,
114133
"__builtin_unreachable", NULL,
@@ -132,6 +151,16 @@ class BuiltinsContext
132151
0);
133152
}
134153

154+
static void handle_flags (tree decl, int flags)
155+
{
156+
if (flags & builtin_const)
157+
TREE_READONLY (decl) = 1;
158+
if (flags & builtin_noreturn)
159+
TREE_READONLY (decl) = 1;
160+
if (flags & builtin_novops)
161+
DECL_IS_NOVOPS (decl) = 1;
162+
}
163+
135164
// Define a builtin function. BCODE is the builtin function code
136165
// defined by builtins.def. NAME is the name of the builtin function.
137166
// LIBNAME is the name of the corresponding library function, and is
@@ -144,24 +173,16 @@ class BuiltinsContext
144173
{
145174
tree decl = add_builtin_function (name, fntype, bcode, BUILT_IN_NORMAL,
146175
libname, NULL_TREE);
147-
if ((flags & builtin_const) != 0)
148-
TREE_READONLY (decl) = 1;
149-
if ((flags & builtin_noreturn) != 0)
150-
TREE_THIS_VOLATILE (decl) = 1;
151-
if ((flags & builtin_novops) != 0)
152-
DECL_IS_NOVOPS (decl) = 1;
176+
handle_flags (decl, flags);
153177
set_builtin_decl (bcode, decl, true);
178+
154179
this->builtin_functions_[name] = decl;
155180
if (libname != NULL)
156181
{
157182
decl = add_builtin_function (libname, fntype, bcode, BUILT_IN_NORMAL,
158183
NULL, NULL_TREE);
159-
if ((flags & builtin_const) != 0)
160-
TREE_READONLY (decl) = 1;
161-
if ((flags & builtin_noreturn) != 0)
162-
TREE_THIS_VOLATILE (decl) = 1;
163-
if ((flags & builtin_novops) != 0)
164-
DECL_IS_NOVOPS (decl) = 1;
184+
handle_flags (decl, flags);
185+
165186
this->builtin_functions_[libname] = decl;
166187
}
167188

gcc/rust/backend/rust-compile-expr.cc

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "rust-compile-block.h"
2727
#include "rust-compile-implitem.h"
2828
#include "rust-constexpr.h"
29+
#include "rust-gcc.h"
2930

3031
#include "fold-const.h"
3132
#include "realmpfr.h"
@@ -146,9 +147,26 @@ CompileExpr::visit (HIR::ArithmeticOrLogicalExpr &expr)
146147
return;
147148
}
148149

149-
translated
150-
= ctx->get_backend ()->arithmetic_or_logical_expression (op, lhs, rhs,
151-
expr.get_locus ());
150+
if (ctx->in_fn () && !ctx->const_context_p ())
151+
{
152+
auto receiver_tmp = NULL_TREE;
153+
auto receiver
154+
= ctx->get_backend ()->temporary_variable (ctx->peek_fn ().fndecl,
155+
NULL_TREE, TREE_TYPE (lhs),
156+
lhs, true, expr.get_locus (),
157+
&receiver_tmp);
158+
auto check
159+
= ctx->get_backend ()->arithmetic_or_logical_expression_checked (
160+
op, lhs, rhs, expr.get_locus (), receiver);
161+
162+
ctx->add_statement (check);
163+
translated = receiver->get_tree (expr.get_locus ());
164+
}
165+
else
166+
{
167+
translated = ctx->get_backend ()->arithmetic_or_logical_expression (
168+
op, lhs, rhs, expr.get_locus ());
169+
}
152170
}
153171

154172
void
@@ -176,13 +194,27 @@ CompileExpr::visit (HIR::CompoundAssignmentExpr &expr)
176194
return;
177195
}
178196

179-
auto operator_expr
180-
= ctx->get_backend ()->arithmetic_or_logical_expression (op, lhs, rhs,
181-
expr.get_locus ());
182-
tree assignment
183-
= ctx->get_backend ()->assignment_statement (lhs, operator_expr,
184-
expr.get_locus ());
185-
ctx->add_statement (assignment);
197+
if (ctx->in_fn () && !ctx->const_context_p ())
198+
{
199+
auto tmp = NULL_TREE;
200+
auto receiver
201+
= ctx->get_backend ()->temporary_variable (ctx->peek_fn ().fndecl,
202+
NULL_TREE, TREE_TYPE (lhs),
203+
lhs, true, expr.get_locus (),
204+
&tmp);
205+
auto check
206+
= ctx->get_backend ()->arithmetic_or_logical_expression_checked (
207+
op, lhs, rhs, expr.get_locus (), receiver);
208+
ctx->add_statement (check);
209+
210+
translated = ctx->get_backend ()->assignment_statement (
211+
lhs, receiver->get_tree (expr.get_locus ()), expr.get_locus ());
212+
}
213+
else
214+
{
215+
translated = ctx->get_backend ()->arithmetic_or_logical_expression (
216+
op, lhs, rhs, expr.get_locus ());
217+
}
186218
}
187219

188220
void
@@ -2378,7 +2410,10 @@ CompileExpr::array_copied_expr (Location expr_locus,
23782410
return error_mark_node;
23792411
}
23802412

2413+
ctx->push_const_context ();
23812414
tree capacity_expr = CompileExpr::Compile (elems.get_num_copies_expr (), ctx);
2415+
ctx->pop_const_context ();
2416+
23822417
if (!TREE_CONSTANT (capacity_expr))
23832418
{
23842419
rust_error_at (expr_locus, "non const num copies %qT", array_type);

gcc/rust/backend/rust-compile-item.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ CompileItem::visit (HIR::Function &function)
160160
function.get_mappings ().get_nodeid (), &canonical_path);
161161
rust_assert (ok);
162162

163+
if (function.get_qualifiers ().is_const ())
164+
ctx->push_const_context ();
165+
163166
tree fndecl
164167
= compile_function (ctx, function.get_function_name (),
165168
function.get_self_param (),
@@ -169,6 +172,9 @@ CompileItem::visit (HIR::Function &function)
169172
function.get_definition ().get (), canonical_path,
170173
fntype, function.has_function_return_type ());
171174
reference = address_expression (fndecl, ref_locus);
175+
176+
if (function.get_qualifiers ().is_const ())
177+
ctx->pop_const_context ();
172178
}
173179

174180
void

gcc/rust/backend/rust-compile-type.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,11 @@ TyTyResolveCompile::visit (const TyTy::ArrayType &type)
370370
{
371371
tree element_type
372372
= TyTyResolveCompile::compile (ctx, type.get_element_type ());
373+
374+
ctx->push_const_context ();
373375
tree capacity_expr = CompileExpr::Compile (&type.get_capacity_expr (), ctx);
376+
ctx->pop_const_context ();
377+
374378
tree folded_capacity_expr = fold_expr (capacity_expr);
375379

376380
translated

gcc/rust/checks/lints/rust-lint-scan-deadcode.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class ScanDeadcode : public MarkLiveBase
5353
void visit (HIR::Function &function) override
5454
{
5555
HirId hirId = function.get_mappings ().get_hirid ();
56-
if (should_warn (hirId))
56+
if (should_warn (hirId) && !function.get_visibility ().is_public ())
5757
{
5858
if (mappings->is_impl_item (hirId))
5959
{
@@ -78,7 +78,7 @@ class ScanDeadcode : public MarkLiveBase
7878
void visit (HIR::StructStruct &stct) override
7979
{
8080
HirId hirId = stct.get_mappings ().get_hirid ();
81-
if (should_warn (hirId))
81+
if (should_warn (hirId) && !stct.get_visibility ().is_public ())
8282
{
8383
bool name_starts_underscore = stct.get_identifier ().at (0) == '_';
8484
if (!name_starts_underscore)
@@ -92,7 +92,8 @@ class ScanDeadcode : public MarkLiveBase
9292
for (auto &field : stct.get_fields ())
9393
{
9494
HirId field_hir_id = field.get_mappings ().get_hirid ();
95-
if (should_warn (field_hir_id))
95+
if (should_warn (field_hir_id)
96+
&& !field.get_visibility ().is_public ())
9697
{
9798
rust_warning_at (field.get_locus (), 0,
9899
"field is never read: %<%s%>",
@@ -106,7 +107,7 @@ class ScanDeadcode : public MarkLiveBase
106107
{
107108
// only warn tuple struct unconstructed, and ignoring unused field
108109
HirId hirId = stct.get_mappings ().get_hirid ();
109-
if (should_warn (hirId))
110+
if (should_warn (hirId) && !stct.get_visibility ().is_public ())
110111
{
111112
rust_warning_at (stct.get_locus (), 0,
112113
"struct is never constructed: %<%s%>",

gcc/rust/rust-backend.h

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,13 +235,24 @@ class Backend
235235
// Supported values of OP are enumerated in ArithmeticOrLogicalOperator.
236236
virtual tree arithmetic_or_logical_expression (ArithmeticOrLogicalOperator op,
237237
tree left, tree right,
238-
Location)
238+
Location loc)
239+
= 0;
240+
241+
// Return an expression for the operation LEFT OP RIGHT.
242+
// Supported values of OP are enumerated in ArithmeticOrLogicalOperator.
243+
// This function adds overflow checking and returns a list of statements to
244+
// add to the current function context. The `receiver` variable refers to the
245+
// variable which will contain the result of that operation.
246+
virtual tree
247+
arithmetic_or_logical_expression_checked (ArithmeticOrLogicalOperator op,
248+
tree left, tree right, Location loc,
249+
Bvariable *receiver)
239250
= 0;
240251

241252
// Return an expression for the operation LEFT OP RIGHT.
242253
// Supported values of OP are enumerated in ComparisonOperator.
243254
virtual tree comparison_expression (ComparisonOperator op, tree left,
244-
tree right, Location)
255+
tree right, Location loc)
245256
= 0;
246257

247258
// Return an expression for the operation LEFT OP RIGHT.
@@ -416,8 +427,8 @@ class Backend
416427
// variable, and may not be very useful. This function should
417428
// return a variable which can be referenced later and should set
418429
// *PSTATEMENT to a statement which initializes the variable.
419-
virtual Bvariable *temporary_variable (tree, tree, tree, tree init,
420-
bool address_is_taken,
430+
virtual Bvariable *temporary_variable (tree fndecl, tree bind_tree, tree type,
431+
tree init, bool address_is_taken,
421432
Location location, tree *pstatement)
422433
= 0;
423434

gcc/rust/rust-diagnostics.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,17 @@ rust_error_at (const Location location, const char *fmt, ...)
166166
va_end (ap);
167167
}
168168

169+
void
170+
rust_error_at (const RichLocation &location, const ErrorCode code,
171+
const char *fmt, ...)
172+
{
173+
va_list ap;
174+
175+
va_start (ap, fmt);
176+
rust_be_error_at (location, code, expand_message (fmt, ap));
177+
va_end (ap);
178+
}
179+
169180
void
170181
rust_warning_at (const Location location, int opt, const char *fmt, ...)
171182
{

0 commit comments

Comments
 (0)