From fd8f1772347d122b223ef573aeaa34cfa93ceec5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 10 Jul 2020 09:46:38 +1000 Subject: [PATCH 1/2] Enforce the static symbol order. By making the proc macro abort if any symbols are out of order. The commit also changes the proc macro collect multiple errors (of order or duplicated symbols) and prints them at the end, which is useful if you have multiple errors. --- src/librustc_macros/src/symbols.rs | 29 +++- src/librustc_span/symbol.rs | 223 +++++++++++++++-------------- 2 files changed, 140 insertions(+), 112 deletions(-) diff --git a/src/librustc_macros/src/symbols.rs b/src/librustc_macros/src/symbols.rs index 0f8bc4323070b..2e9b3a2a2562f 100644 --- a/src/librustc_macros/src/symbols.rs +++ b/src/librustc_macros/src/symbols.rs @@ -87,18 +87,29 @@ pub fn symbols(input: TokenStream) -> TokenStream { let mut prefill_stream = quote! {}; let mut counter = 0u32; let mut keys = HashSet::::new(); + let mut prev_key: Option = None; + let mut errors = Vec::::new(); - let mut check_dup = |str: &str| { + let mut check_dup = |str: &str, errors: &mut Vec| { if !keys.insert(str.to_string()) { - panic!("Symbol `{}` is duplicated", str); + errors.push(format!("Symbol `{}` is duplicated", str)); } }; + let mut check_order = |str: &str, errors: &mut Vec| { + if let Some(ref prev_str) = prev_key { + if str < prev_str { + errors.push(format!("Symbol `{}` must precede `{}`", str, prev_str)); + } + } + prev_key = Some(str.to_string()); + }; + // Generate the listed keywords. for keyword in &input.keywords.0 { let name = &keyword.name; let value = &keyword.value; - check_dup(&value.value()); + check_dup(&value.value(), &mut errors); prefill_stream.extend(quote! { #value, }); @@ -116,7 +127,8 @@ pub fn symbols(input: TokenStream) -> TokenStream { Some(value) => value.value(), None => name.to_string(), }; - check_dup(&value); + check_dup(&value, &mut errors); + check_order(&name.to_string(), &mut errors); prefill_stream.extend(quote! { #value, }); @@ -131,7 +143,7 @@ pub fn symbols(input: TokenStream) -> TokenStream { // Generate symbols for the strings "0", "1", ..., "9". for n in 0..10 { let n = n.to_string(); - check_dup(&n); + check_dup(&n, &mut errors); prefill_stream.extend(quote! { #n, }); @@ -141,6 +153,13 @@ pub fn symbols(input: TokenStream) -> TokenStream { counter += 1; } + if !errors.is_empty() { + for error in errors.into_iter() { + eprintln!("error: {}", error) + } + panic!("errors in `Keywords` and/or `Symbols`"); + } + let tt = TokenStream::from(quote! { macro_rules! keywords { () => { diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 86b770104ea8e..277ca8fa5b1ac 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -19,6 +19,7 @@ use crate::{Span, DUMMY_SP, SESSION_GLOBALS}; #[cfg(test)] mod tests; +// The proc macro code for this is in `src/librustc_macros/src/symbols.rs`. symbols! { // After modifying this list adjust `is_special`, `is_used_keyword`/`is_unused_keyword`, // this should be rarely necessary though if the keywords are kept in alphabetic order. @@ -113,8 +114,78 @@ symbols! { // As well as the symbols listed, there are symbols for the the strings // "0", "1", ..., "9", which are accessible via `sym::integer`. // - // Keep this list in sorted order, as defined by the Unix `sort` utility. + // The proc macro will abort if symbols are not in alphabetical order (as + // defined by `impl Ord for str`) or if any symbols are duplicated. Vim + // users can sort the list by selecting it and executing the command + // `:'<,'>!LC_ALL=C sort`. + // + // There is currently no checking that all symbols are used; that would be + // nice to have. Symbols { + Arc, + ArgumentV1, + Arguments, + C, + Clone, + Copy, + Debug, + Decodable, + Default, + Encodable, + Eq, + Equal, + Err, + From, + Future, + FxHashMap, + FxHashSet, + GlobalAlloc, + Hash, + HashMap, + HashSet, + Input, + IntoIterator, + Iterator, + Layout, + LintPass, + None, + Ok, + Option, + Ord, + Ordering, + Output, + PartialEq, + PartialOrd, + Pending, + Pin, + Poll, + ProcMacroHack, + ProceduralMasqueradeDummyType, + Range, + RangeFrom, + RangeFull, + RangeInclusive, + RangeTo, + RangeToInclusive, + Rc, + Ready, + Result, + Return, + RustcDecodable, + RustcEncodable, + Send, + Some, + Sync, + Target, + Try, + Ty, + TyCtxt, + TyKind, + Vec, + Yield, + _Self, + __next, + _task_context, aarch64_target_feature, abi, abi_amdgpu_kernel, @@ -131,8 +202,8 @@ symbols! { aborts, add, add_assign, - address, add_with_overflow, + address, advanced_slice_patterns, adx_target_feature, alias, @@ -141,28 +212,26 @@ symbols! { alignstack, all, alloc, - allocator, - allocator_internals, alloc_error_handler, alloc_layout, alloc_zeroed, + allocator, + allocator_internals, allow, - allowed, allow_fail, allow_internal_unsafe, allow_internal_unstable, allow_internal_unstable_backcompat_hack, + allowed, always, and, any, arbitrary_enum_discriminant, arbitrary_self_types, - Arc, - Arguments, - ArgumentV1, arith_offset, arm_target_feature, array, + as_str, asm, assert, assert_inhabited, @@ -173,16 +242,15 @@ symbols! { associated_type_bounds, associated_type_defaults, associated_types, - as_str, assume, assume_init, async_await, async_closure, atomics, + att_syntax, attr, - attributes, attr_literals, - att_syntax, + attributes, augmented_assignments, automatically_derived, avx512_target_feature, @@ -210,11 +278,11 @@ symbols! { braced_empty_structs, breakpoint, bswap, - C, + c_variadic, call, - caller_location, call_mut, call_once, + caller_location, cdylib, ceilf32, ceilf64, @@ -232,7 +300,6 @@ symbols! { char, clippy, clone, - Clone, clone_closures, clone_from, closure_to_fn_coercion, @@ -274,7 +341,6 @@ symbols! { context, convert, copy, - Copy, copy_closures, copy_nonoverlapping, copysignf32, @@ -303,18 +369,14 @@ symbols! { custom_derive, custom_inner_attributes, custom_test_frameworks, - c_variadic, dead_code, dealloc, debug, - Debug, debug_assertions, debug_trait, - declare_lint_pass, decl_macro, - Decodable, + declare_lint_pass, decode, - Default, default_lib_allocator, default_type_parameter_fallback, default_type_params, @@ -339,8 +401,8 @@ symbols! { doc_masked, doctest, document_private_items, - dotdoteq_in_patterns, dotdot_in_tuple_patterns, + dotdoteq_in_patterns, double_braced_closure: "{{closure}}", double_braced_constant: "{{constant}}", double_braced_constructor: "{{constructor}}", @@ -349,24 +411,20 @@ symbols! { double_braced_misc: "{{misc}}", double_braced_opaque: "{{opaque}}", drop, - dropck_eyepatch, - dropck_parametricity, drop_in_place, drop_types_in_const, + dropck_eyepatch, + dropck_parametricity, dylib, dyn_trait, eh_catch_typeinfo, eh_personality, enable, enclosing_scope, - Encodable, encode, env, eq, - Eq, - Equal, err, - Err, exact_div, except, exchange_malloc, @@ -382,12 +440,12 @@ symbols! { export_name, expr, extern_absolute_paths, - external_doc, extern_crate_item_prelude, extern_crate_self, extern_in_paths, extern_prelude, extern_types, + external_doc, f16c_target_feature, f32, f32_runtime, @@ -424,7 +482,6 @@ symbols! { freeze, frem_fast, from, - From, from_desugaring, from_error, from_generator, @@ -436,29 +493,22 @@ symbols! { fsub_fast, fundamental, future, - Future, future_trait, - FxHashMap, - FxHashSet, ge, + gen_future, + gen_kill, generator, - generators, generator_state, + generators, generic_associated_types, generic_param_attrs, - gen_future, - gen_kill, get_context, - GlobalAlloc, global_allocator, global_asm, globs, gt, half_open_range_patterns, hash, - Hash, - HashMap, - HashSet, hexagon_target_feature, hidden, homogeneous_aggregate, @@ -493,10 +543,8 @@ symbols! { inlateout, inline, inout, - Input, intel, into_iter, - IntoIterator, into_result, intrinsics, irrefutable_let_patterns, @@ -508,7 +556,6 @@ symbols! { item_context: "ItemContext", item_like_imports, iter, - Iterator, keyword, kind, label, @@ -516,7 +563,6 @@ symbols! { lang, lang_items, lateout, - Layout, lazy_normalization_consts, le, let_chains, @@ -527,14 +573,13 @@ symbols! { likely, line, link, - linkage, link_args, link_cfg, link_llvm_intrinsics, link_name, link_ordinal, link_section, - LintPass, + linkage, lint_reasons, literal, llvm_asm, @@ -543,9 +588,9 @@ symbols! { log10f64, log2f32, log2f64, + log_syntax, logf32, logf64, - log_syntax, loop_break_value, lt, macro_at_most_once_rep, @@ -554,9 +599,9 @@ symbols! { macro_lifetime_matcher, macro_literal_matcher, macro_reexport, - macros_in_extern, macro_use, macro_vis_matcher, + macros_in_extern, main, managed_boxes, manually_drop, @@ -567,23 +612,23 @@ symbols! { match_default_bindings, maxnumf32, maxnumf64, + may_dangle, maybe_uninit, maybe_uninit_uninit, maybe_uninit_zeroed, - may_dangle, - member_constraints, - memory, mem_uninitialized, mem_zeroed, + member_constraints, + memory, message, meta, min_align_of, min_align_of_val, min_const_fn, min_const_unsafe_fn, + min_specialization, minnumf32, minnumf64, - min_specialization, mips_target_feature, miri_start_panic, mmx_target_feature, @@ -615,7 +660,6 @@ symbols! { never_type, never_type_fallback, new, - __next, next, nll, no, @@ -629,47 +673,41 @@ symbols! { no_link, no_main, no_mangle, + no_niche, + no_sanitize, + no_stack_check, + no_start, + no_std, nomem, non_ascii_idents, - None, - none_error, non_exhaustive, - no_niche, non_modrs_mods, + none_error, nontemporal_store, nontrapping_dash_fptoint: "nontrapping-fptoint", noreturn, - no_sanitize, nostack, - no_stack_check, - no_start, - no_std, not, note, object_safe_for_dispatch, offset, - Ok, omit_gdb_pretty_printer_section, on, on_unimplemented, oom, opaque, ops, + opt_out_copy, optimize, optimize_attribute, optin_builtin_traits, option, - Option, option_env, - options, option_type, - opt_out_copy, + options, or, - Ord, - Ordering, or_patterns, out, - Output, overlapping_marker_traits, owned_box, packed, @@ -686,17 +724,13 @@ symbols! { param_attrs, parent_trait, partial_cmp, - PartialEq, partial_ord, - PartialOrd, passes, pat, path, pattern_parentheses, - Pending, phantom_data, pin, - Pin, pinned, platform_intrinsics, plugin, @@ -704,15 +738,14 @@ symbols! { plugins, pointer, poll, - Poll, post_dash_lto: "post-lto", powerpc_target_feature, powf32, powf64, powif32, powif64, - precise_pointer_size_matching, pre_dash_lto: "pre-lto", + precise_pointer_size_matching, pref_align_of, prefetch_read_data, prefetch_read_instruction, @@ -723,14 +756,12 @@ symbols! { preserves_flags, primitive, proc_dash_macro: "proc-macro", - ProceduralMasqueradeDummyType, proc_macro, proc_macro_attribute, proc_macro_def_site, proc_macro_derive, proc_macro_expr, proc_macro_gen, - ProcMacroHack, proc_macro_hygiene, proc_macro_internals, proc_macro_mod, @@ -747,18 +778,11 @@ symbols! { quad_precision_float, question_mark, quote, - Range, - RangeFrom, - RangeFull, - RangeInclusive, - RangeTo, - RangeToInclusive, raw_dylib, raw_identifiers, raw_ref_op, - Rc, + re_rebalance_coherence, readonly, - Ready, realloc, reason, receiver, @@ -779,11 +803,8 @@ symbols! { repr_packed, repr_simd, repr_transparent, - re_rebalance_coherence, result, - Result, result_type, - Return, rhs, rintf32, rintf64, @@ -799,6 +820,10 @@ symbols! { rust_2015_preview, rust_2018_preview, rust_begin_unwind, + rust_eh_personality, + rust_eh_register_frames, + rust_eh_unregister_frames, + rust_oom, rustc, rustc_allocator, rustc_allocator_nounwind, @@ -810,7 +835,6 @@ symbols! { rustc_const_stable, rustc_const_unstable, rustc_conversion_suggestion, - RustcDecodable, rustc_def_path, rustc_deprecated, rustc_diagnostic_item, @@ -820,7 +844,6 @@ symbols! { rustc_dump_env_program_clauses, rustc_dump_program_clauses, rustc_dump_user_substs, - RustcEncodable, rustc_error, rustc_expected_cgu_reuse, rustc_if_this_changed, @@ -857,19 +880,15 @@ symbols! { rustc_then_this_would_need, rustc_unsafe_specialization_marker, rustc_variance, - rust_eh_personality, rustfmt, - rust_oom, rvalue_static_promotion, sanitize, sanitizer_runtime, saturating_add, saturating_sub, - _Self, self_in_typedefs, self_struct_ctor, semitransparent, - Send, send_trait, shl, shl_assign, @@ -937,9 +956,9 @@ symbols! { sinf32, sinf64, size, - sized, size_of, size_of_val, + sized, slice, slice_alloc, slice_patterns, @@ -947,7 +966,6 @@ symbols! { slice_u8_alloc, slicing_syntax, soft, - Some, specialization, speed, sqrtf32, @@ -957,9 +975,9 @@ symbols! { staged_api, start, static_in_const, - staticlib, static_nobundle, static_recursion, + staticlib, std, std_inject, stmt, @@ -970,10 +988,10 @@ symbols! { stringify, struct_field_attributes, struct_inherit, + struct_variant, structural_match, structural_peq, structural_teq, - struct_variant, sty, sub, sub_assign, @@ -981,9 +999,7 @@ symbols! { suggestion, sym, sync, - Sync, sync_trait, - Target, target_arch, target_endian, target_env, @@ -998,7 +1014,6 @@ symbols! { target_thread_local, target_vendor, task, - _task_context, tbm_target_feature, termination, termination_trait, @@ -1024,7 +1039,6 @@ symbols! { trivial_bounds, truncf32, truncf64, - Try, try_blocks, try_trait, tt, @@ -1032,9 +1046,6 @@ symbols! { tuple_indexing, two_phase, ty, - Ty, - TyCtxt, - TyKind, type_alias_enum_variants, type_alias_impl_trait, type_ascription, @@ -1082,21 +1093,20 @@ symbols! { unwind, unwind_attributes, unwrap_or, - used, use_extern_macros, use_nested_groups, + used, usize, v1, va_arg, va_copy, va_end, - val, va_list, + va_start, + val, var, variant_count, - va_start, vec, - Vec, vec_type, version, vis, @@ -1117,7 +1127,6 @@ symbols! { wrapping_mul, wrapping_sub, write_bytes, - Yield, } } From 600b8247a8008b8e4fb2f6b6f61207ba85aba363 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 10 Jul 2020 10:14:55 +1000 Subject: [PATCH 2/2] Rename `sym::item_context` as `sym::ItemContext`. Because it represents the symbol `ItemContext`, and `sym` identifiers are supposed to match the actual symbol whenever possible. --- src/librustc_span/symbol.rs | 2 +- .../traits/error_reporting/on_unimplemented.rs | 2 +- src/librustc_trait_selection/traits/on_unimplemented.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 277ca8fa5b1ac..75f588918a020 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -145,6 +145,7 @@ symbols! { HashSet, Input, IntoIterator, + ItemContext, Iterator, Layout, LintPass, @@ -553,7 +554,6 @@ symbols! { issue_5723_bootstrap, issue_tracker_base_url, item, - item_context: "ItemContext", item_like_imports, iter, keyword, diff --git a/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs b/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs index ec51dddc2c810..d2b9f84af33ae 100644 --- a/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs +++ b/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs @@ -126,7 +126,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let mut flags = vec![]; flags.push(( - sym::item_context, + sym::ItemContext, self.describe_enclosure(obligation.cause.body_id).map(|s| s.to_owned()), )); diff --git a/src/librustc_trait_selection/traits/on_unimplemented.rs b/src/librustc_trait_selection/traits/on_unimplemented.rs index a1dfa838e7af3..deb33708681fa 100644 --- a/src/librustc_trait_selection/traits/on_unimplemented.rs +++ b/src/librustc_trait_selection/traits/on_unimplemented.rs @@ -286,7 +286,7 @@ impl<'tcx> OnUnimplementedFormatString { // `{from_desugaring}` is allowed Position::ArgumentNamed(s) if s == sym::from_desugaring => (), // `{ItemContext}` is allowed - Position::ArgumentNamed(s) if s == sym::item_context => (), + Position::ArgumentNamed(s) if s == sym::ItemContext => (), // So is `{A}` if A is a type parameter Position::ArgumentNamed(s) => { match generics.params.iter().find(|param| param.name == s) { @@ -350,7 +350,7 @@ impl<'tcx> OnUnimplementedFormatString { let s = self.0.as_str(); let parser = Parser::new(&s, None, None, false, ParseMode::Format); - let item_context = (options.get(&sym::item_context)).unwrap_or(&empty_string); + let item_context = (options.get(&sym::ItemContext)).unwrap_or(&empty_string); parser .map(|p| match p { Piece::String(s) => s, @@ -364,7 +364,7 @@ impl<'tcx> OnUnimplementedFormatString { } else if s == sym::from_desugaring || s == sym::from_method { // don't break messages using these two arguments incorrectly &empty_string - } else if s == sym::item_context { + } else if s == sym::ItemContext { &item_context } else { bug!(