[RFC] Changing definition of Comment

[kdy1]

I'm considering changing Comment to store Arc<String> instead of String.
This is because comments are typically simply passed around, and there are lots of cases where comments should be cloned.
Also, it's quite hard to modify the content of Comment without allocation, so it's not likely to degrade performance.

[kdy1]

cc @dsherret @bartlomieju
cc @devongovett @mischnic

Do you use some methods like to_ascii_lowercase or something similar method, where the output has same length as input? (If length is different, you are re-allocating anyway)

[dsherret]

When do you need to clone them? Is this something that could be done if desired after parsing? I think it’s not a big deal whatever is decided.

[kdy1]

I needed to clone them while translating ast into a different format.

[kdy1]

Also, some comments like PURE are cloned lots of times.

[dsherret]

One possibility is after parsing, the comments could be taken via something like take_all (

swc/crates/swc_common/src/comments.rs

Line 490 in db67435

pub fn take_all(self) -> (SingleThreadedCommentsMap, SingleThreadedCommentsMap) {

) and then mapped to be in an Arc if necessary at that point. Whatever is decided should be ok for us so no big deal whatever way.

By the way, maybe #1952 should be thought about at the same time. Perhaps passing around Arc<Comment> would reduce the cloning necessary in that API (ex. for get_x methods

swc/crates/swc_common/src/comments.rs

Line 41 in db67435

fn get_trailing(&self, pos: BytePos) -> Option<Vec<Comment>>;

).

Edit: Sorry, I’m on my phone too lazy to fix my messy comment 😅

[kdy1]

Oh, yes you are right. I can change type when necessary.

Also yes, I forgot to mention it, but actually the api of Comments is also problematic because of cloning.