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.

Sign up for GitHub

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

Jump to bottom

Remove Rust::make_unique #3278

Merged
merged 1 commit into from
Jan 3, 2025
Merged

Remove Rust::make_unique #3278

merged 1 commit into from
Jan 3, 2025

Conversation

powerboat9
Copy link
Contributor

While minor, it looks like this fix was accidentally applied while upstreaming and was never performed downstream

@powerboat9
Copy link
Contributor Author

It looks like my clang-format might be misbehaving. Alternatively, we could push an inverted patch upstream, or leave upstream and downstream out of sync. A PR to remove our implementation of std::make_unique (now that C++14 is supported) could make the formatting moot, but would be non-trivial to upstream while we're out of sync.

@philberty
Copy link
Member

I think this is all my fault my desktop is a super old clang format 11 or something stupid iirc. What version of clang format are you using @CohenArthur and @P-E-P ? Maybe we look to upgrade and I will sort my life out lol

@P-E-P
Copy link
Member

P-E-P commented Dec 2, 2024

I think this is all my fault my desktop is a super old clang format 11 or something stupid iirc. What version of clang format are you using @CohenArthur and @P-E-P ? Maybe we look to upgrade and I will sort my life out lol

IIRC we should use clang format 10 because newer versions have some incompatibilities with their configuration.

@powerboat9
Copy link
Contributor Author

I'm leaning towards just merging this PR, since it's already in effect upstream -- from conversations on the #gcc IRC channel it looks like the proper formatting here is a grey area

CohenArthur
CohenArthur approved these changes Dec 4, 2024
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with merging this. I think there are lots of grey areas when it comes to formatting C++ code for GCC. I'm happy as long as clang-format is happy, and variadics are the devil's weapon anyway so they don't deserve nice formatting.

@CohenArthur
Copy link
Member

Ah, but clang-format isn't happy... Hmmm. This is annoying since it's going to trigger on all future PRs, so we can't merge it. We can update our clang-format to use a more recent version, as Philip pointed out we're supposed to use 10 for the project but it's 9 versions behind.

@powerboat9
Copy link
Contributor Author

I could use // clang-format {on,off}, but I'd probably want to stick that in a separate comment for ease of upstreaming later

@powerboat9
Copy link
Contributor Author

I think it might just be better to remove this file, now that we have std::make_unique with C++14

@powerboat9 powerboat9 changed the title Fix make_unique formatting Remove Rust::make_unique Dec 4, 2024
@powerboat9
Copy link
Contributor Author

It looks like both of the failing builds are using -std=gnu++11

@thesamesam
Copy link
Contributor

thesamesam commented Dec 5, 2024 •
edited
Loading

I think that might be because we need a rebase. That is, we need r15-4719-ga9ec1bc06bd3cc.

EDIT: See #3294.

@thesamesam thesamesam mentioned this pull request Dec 5, 2024
Merged
@CohenArthur
Copy link
Member

Yeah, let's wait for #3294 or for an update from upstream. I'm happy with removing our make_unique.

@thesamesam
Copy link
Contributor

#3294 has now landed so a rebase should have green CI.

@powerboat9
Copy link
Contributor Author

Collecting some stats on gcc using a minimally efficient bash script, but I should be able to rebase in a bit

2024-12-11-162451_817x482_scrot

@powerboat9
Copy link
Contributor Author

powerboat9 commented Dec 12, 2024 •
edited
Loading

This should work now

Edit: looks like I'll need to remove some more Rust::make_unique usages

@powerboat9
Copy link
Contributor Author

It looks like there's an issue with the clang check

@thesamesam
Copy link
Contributor 

 checking dependency style of clang++ -std=gnu++14... clang++ -std=gnu++14  -I../../libcpp -I. -I../../libcpp/../include -I/usr/local/include -I../../libcpp/include -I/usr/local/include -g -O2     -W -Wall -Wno-narrowing -Wwrite-strings -Wmissing-format-attribute -pedantic -Wno-long-long  -fno-exceptions -fno-rtti -I../../libcpp -I. -I../../libcpp/../include -I/usr/local/include -I../../libcpp/include -I/usr/local/include   -c -o charset.o -MT charset.o -MMD -MP -MF .deps/charset.Tpo ../../libcpp/charset.cc
gcc3
checking how to run the C++ preprocessor... In file included from ../../libcpp/charset.cc:21:
In file included from ../../libcpp/system.h:279:
/usr/local/include/libintl.h:1089:34: error: expected unqualified-id
char *setlocale (int __category, const char *__locale)
                                 ^
/usr/local/include/libintl.h:1089:34: error: expected ')'
/usr/local/include/libintl.h:1089:7: note: to match this '('
char *setlocale (int __category, const char *__locale)
      ^
../../libcpp/system.h:275:38: note: expanded from macro 'setlocale'
# define setlocale(category, locale) (locale)
   `                                  ^

We may need to install libintl from brew as well.

@powerboat9
Copy link
Contributor Author

@CohenArthur @philberty

@powerboat9
Remove Rust::make_unique
df115f3 
Since our bootstrap requirement has been bumped to C++14, we don't need
a custom implementation of std::make_unique anymore.

gcc/rust/ChangeLog:

	* ast/rust-ast-builder-type.cc: Remove inclusion of
	rust-make-unique.h.
	* ast/rust-ast-builder.cc: Likewise.
	(Builder::array): Use std::make_unique instead of
	Rust::make_unique.
	* ast/rust-ast.cc (Attribute::get_traits_to_derive): Likewise.
	* ast/rust-macro.h: Remove inclusion of rust-make-unique.h.
	(MacroRulesDefinition::mbe): Use std::make_unique instead of
	Rust::make_unique.
	(MacroRulesDefinition::decl_macro): Likewise.
	* ast/rust-path.h
	(PathInExpression::PathInExpression): Likewise.
	(QualifiedPathInExpression::QualifiedPathInExpression):
	Likewise.
	* backend/rust-compile-pattern.cc
	(CompilePatternCheckExpr::visit): Likewise.
	* expand/rust-derive-copy.cc
	(DeriveCopy::copy_impl): Likewise.
	* expand/rust-expand-format-args.cc
	(expand_format_args): Likewise.
	* expand/rust-macro-builtins-asm.cc: Remove inclusion of
	rust-make-unique.h.
	(parse_asm): Use std::make_unique instead of Rust::make_unique.
	* hir/rust-ast-lower-expr.cc
	(ASTLoweringExpr::visit): Likewise.
	* hir/tree/rust-hir-expr.cc
	(StructExprStructFields::StructExprStructFields): Likewise.
	(StructExprStructFields::operator=): Likewise.
	* hir/tree/rust-hir.cc
	(TypePath::to_trait_bound): Likewise.
	* lex/rust-token.h: Remove inclusion of rust-make-unique.h.
	(Token::Token): Use std::make_unique instead of
	Rust::make_unique.
	* metadata/rust-import-archive.cc: Remove inclusion of
	rust-make-unique.h.
	(Import::find_archive_export_data): Use std::make_unique instead
	of Rust::make_unique.
	* metadata/rust-imports.cc: Remove inclusion of
	rust-make-unique.h.
	(Import::find_export_data): Use std::make_unique instead of
	Rust::make_unique.
	(Import::find_object_export_data): Likewise.
	* parse/rust-parse-impl.h: Remove inclusion of
	rust-make-unique.h.
	(Parser::parse_function_param): Use std::make_unique instead of
	Rust::make_unique.
	(Parser::parse_self_param): Likewise.
	(Parser::parse_array_expr): Likewise.
	* typecheck/rust-hir-type-check-enumitem.cc
	(TypeCheckEnumItem::visit): Likewise.
	* typecheck/rust-hir-type-check-implitem.cc
	(TypeCheckTopLevelExternItem::visit): Likewise.
	(TypeCheckImplItem::visit): Likewise.
	* typecheck/rust-hir-type-check-type.cc
	(TypeResolveGenericParam::visit): Likewise.
	* typecheck/rust-hir-type-check.cc: Remove inclusion of
	rust-make-unique.h.
	(TraitItemReference::get_type_from_fn): Use std::make_unique
	instead of Rust::make_unique.
	* typecheck/rust-tyty-bounds.cc
	(TypeCheckBase::get_predicate_from_bound): Likewise.
	* util/rust-make-unique.h: Removed.

Signed-off-by: Owen Avery <[email protected]>
@CohenArthur CohenArthur added this pull request to the merge queue Jan 3, 2025
Merged via the queue into Rust-GCC:master with commit de8606f Jan 3, 2025
12 checks passed
@powerboat9 powerboat9 deleted the fix-fmt branch January 3, 2025 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur Awaiting requested review from CohenArthur

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@powerboat9 @philberty @P-E-P @CohenArthur @thesamesam