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.

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

discard trailing blank comments #2925

Merged
merged 3 commits into from
Aug 22, 2018
Merged

discard trailing blank comments #2925

merged 3 commits into from
Aug 22, 2018

Conversation

scampi
Copy link
Contributor

@scampi scampi commented Aug 16, 2018

close #539

scampi added 3 commits August 16, 2018 19:55
Block comments like below were not properly supported:

    /*
       something here but it doesn't start with a star
     */

because of the line that didn't start with a star.
@scampi
Copy link
Contributor Author

scampi commented Aug 17, 2018

The packed_simd build fails with the following. However, it is strange that all but the first line of the comment is indented to begin with; I feel like it shouldn't be indented.

It seems related to #2917

Diff in /home/yfful/documents/code/rustfmt/packed_simd/src/api/from/from_vector.rs at line 18:
         }
 
         // FIXME: `Into::into` is not inline, but due to
-                                                // the blanket impl in `std`, which is not
-                                                // marked `default`, we cannot override it here with
-                                                // specialization.
-                                                /*
-                                                impl Into<$id> for $source {
-                                                    #[inline]
-                                                    fn into(self) -> $id {
-                                                        unsafe { simd_cast(self) }
-                                                    }
-                                                }
-                                                */
+                                                        // the blanket impl in `std`, which is not
+                                                        // marked `default`, we cannot override it here with
+                                                        // specialization.
+                                                        /*
+                                                        impl Into<$id> for $source {
+                                                            #[inline]
+                                                            fn into(self) -> $id {
+                                                                unsafe { simd_cast(self) }
+                                                            }
+                                                        }
+                                                        */
 
         test_if!{
             $test_tt:
Diff in /home/yfful/documents/code/rustfmt/packed_simd/src/api/reductions/min_max.rs at line 41:
                     target_arch = "arm",
                     all(target_arch = "x86", not(target_feature = "sse2")),
                     target_arch = "powerpc64",
-                ),))]
+                )))]
                 {
                     use llvm::simd_reduce_min;
                     let v: $ielem_ty = unsafe { simd_reduce_min(self.0) };

@nrc
Copy link
Member

nrc commented Aug 22, 2018

The packed_simd build fails with the following

Is this a new issue? It doesn't look like it should be caused by your patch, but if it is, it would be good not to and to add this as a test.

@nrc
Copy link
Member

nrc commented Aug 22, 2018

And the code looks great, thanks!

@scampi
Copy link
Contributor Author

scampi commented Aug 22, 2018

With the latest packed_simd master, I don't see anymore differences in packed_simd, but it was happening up to commit rust-lang/packed_simd@1a6450d.

It is interesting to note that the indentation of that commented code increased already in a previous commit rust-lang/packed_simd@0cab9e9#diff-a70485da056880cc8fbf5e1bd3cb33fa

Although that indentation shift is not due it seems to a change in this PR, should I open an issue for it though ? The indentation should not increase and stay at the // FIXME column I guess.

The problem with rand still occurs: it's an extra comma:

Diff in /home/yfful/documents/code/rustfmt/rand/src/rngs/entropy.rs at line 290:
         all(target_arch = "wasm32", feature = "stdweb"),
         all(target_arch = "wasm32", feature = "wasm-bindgen"),
     ),
-)))]
+),))]
 type Os = NoSource;
 
 type Custom = NoSource;

@nrc
Copy link
Member

nrc commented Aug 22, 2018

should I open an issue for it though ?

Yes please!

@nrc nrc merged commit 8573df6 into rust-lang:master Aug 22, 2018
@scampi scampi deleted the issue539 branch August 23, 2018 09:23
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.

When converting /* */ to // style comments, don't add blank lines
2 participants