This is an archive of the discontinued LLVM Phabricator instance.

[Clang][NFC] Fix multiline comment prefixes in function headers
ClosedPublic

Authored by saiislam on Oct 11 2021, 6:54 AM.

Details

Summary

Cleanup of D105191 after latest clang-format changes.

Diff Detail

Event Timeline

saiislam created this revision.Oct 11 2021, 6:54 AM
saiislam requested review of this revision.Oct 11 2021, 6:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2021, 6:54 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
thakis added a subscriber: thakis.Oct 11 2021, 12:09 PM
thakis added inline comments.
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
118

The usual style for "named parameters" in LLVm is

AddStaticDeviceLibsLinking(C, *this, JA, Inputs, Args, CmdArgs, "amdgcn",
                           SubArchName, /*isBitCodeSDL=*/true, /*postClangLink=*/false);

what @thakis said, this isn't normally my area but this is one of the files that has reverted its "clang-format clean" status with recent commits.

saiislam updated this revision to Diff 378957.Oct 12 2021, 3:56 AM

Used comment style for named parameters.

MyDeveloperDay requested changes to this revision.Oct 14 2021, 12:26 AM

To me you should be doing

/*bitcode SDL=*/true

It looks like its still failing clang-format might I suggest making the changes to the comment, git adding the file then doing

git clang-format

This will clang-format your staged files.

This revision now requires changes to proceed.Oct 14 2021, 12:26 AM
saiislam updated this revision to Diff 397073.Jan 3 2022, 8:40 AM

Updated comments as per suggestion. Apologies for the delay.

MyDeveloperDay added a comment.EditedJan 4 2022, 12:00 AM

Nit: That is more correct, but actually, you normally make the comment match the param name (I think there might even be a clang-tidy check for that?)

i.e.

/*isBitCodeSDL=*/
MyDeveloperDay accepted this revision.Jan 4 2022, 12:01 AM
This revision is now accepted and ready to land.Jan 4 2022, 12:01 AM
This revision was landed with ongoing or failed builds.Jan 4 2022, 3:53 AM
This revision was automatically updated to reflect the committed changes.

Nit: That is more correct, but actually, you normally make the comment match the param name (I think there might even be a clang-tidy check for that?)

i.e.

/*isBitCodeSDL=*/

Thanks. I fixed it while committing.