Skip to content

More resolution modules with attribute path #1528

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

Merged
merged 3 commits into from
Jul 14, 2019

Conversation

ava57r
Copy link
Contributor

@ava57r ava57r commented Jul 13, 2019

@@ -497,7 +497,7 @@ where

fn collect_module(&mut self, module: &raw::ModuleData) {
match module {
// inline module, just recurse
// inline module, just recursive
Copy link
Member

Choose a reason for hiding this comment

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

I think recurse is correct here and below.

separator(&file_path[current_dir.len()..])
} else {
separator(file_path.as_str())
}
Copy link
Member

Choose a reason for hiding this comment

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

I think one can avoid the closure here:

if cond {
    branch1
} else {
    branch2
}.replace("\\", "/")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

#[test]
fn module_resolution_module_decl_path_super() {
let map = def_map_with_crate_graph(
"
Copy link
Member

Choose a reason for hiding this comment

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

let's use an r#" literal here to avoid escaping " and \ everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -675,6 +676,17 @@ fn resolve_submodule(
}
}

fn normalize_attribute_path(file_path: &SmolStr) -> String {
Copy link
Member

Choose a reason for hiding this comment

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

We could optimisitally try to avoid an allocation here, but, given this is already a cold path, this shouldn't matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@matklad
Copy link
Member

matklad commented Jul 14, 2019

bors r+

bors bot added a commit that referenced this pull request Jul 14, 2019
1528: More resolution modules with attribute path r=matklad a=andreevlex

#1211

Co-authored-by: Alexander Andreev <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 14, 2019

Build succeeded

@bors bors bot merged commit 9c75f30 into rust-lang:master Jul 14, 2019
@ava57r ava57r deleted the feature/more-attr-mod-path branch July 14, 2019 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants