The xpcom/reflect/xptcall/md/ has a bunch of different implementations of some of the lowest level of XPCOM call stuff. Mostly this gets reformatted just fine, but there are a number of macros used to define inline assembly that get reformatted in a way that makes it very hard to understand. For instance, here's an except of the changes to the STUB_ENTRY macro in md/unix/xptcstubs_arm.cpp: -#define STUB_ENTRY(n) \ -nsresult nsXPTCStubBase::Stub##n () \ -{ \ - __asm__ ( \ -" mov ip, #"#n"\n" \ -" b SharedStub\n\t"); \ - return 0; /* avoid warnings */ \ -} +#define STUB_ENTRY(n) \ + nsresult nsXPTCStubBase::Stub##n() { \ + __asm__(" mov ip, #" #n \ + "\n" \ + " b SharedStub\n\t"); \ + return 0; /* avoid warnings */ \ + } The line " mov ip, #"#n"\n", which is a single line of assembly, gets split into multiple lines, which is confusing. This isn't even the worst instance of it. If you want to see a bad one, check out the output for STUB_MANGLED_ENTRY in xptcstubs_alpha_openbsd.cpp. There's clearly been a lot of copy pasting between these files. This issue affects many, but not all, of the inline assembly macros. Files I noticed this issue in are: xptcstubs_alpha_openbsd.cpp xptcstubs_linux_alpha.cpp xptcstubs_arm.cpp xptcstubs_arm_openbsd.cpp xptcstubs_arm_openbsd.cpp xptcstubs_gcc_x86_unix.cpp xptcstubs_ppc64_linux.cpp xptcstubs_ppc_linux.cpp xptcstubs_ppc_openbsd.cpp xptcstubs_x86_64_darwin.cpp xptcstubs_x86_64_linux.cpp Also, from the win32 directory: win32/xptcstubs.cpp win32/xptcstubs_x86_64_gnu.cpp Obviously not all of these are important, but it does seem like some of them are for Tier 1 platforms. The right fix would be to go through all of the macros declared in all of these files and disable clang format for the problematic macros. A hackier fix would just be to disable clang format for everything in these directories. It depends on whether somebody wants to dig through all of these files by Friday. This code is quite crusty so I don't think leaving it alone would be that harmful. It seems better than letting these macros get pushed around like that.

I updated the summary to emphasize that this is about the readability of the output, not the correctness.

Macros that I noticed were affected in these various files are STUB_MANGLED_ENTRY, STUB_ENTRY, STUB_HEADER and STUB_SIZE.

(In reply to Andrew McCreight [:mccr8] from comment #0) > The right fix would be to go through all of the macros declared in all of > these files and disable clang format for the problematic macros. A hackier > fix would just be to disable clang format for everything in these > directories. It depends on whether somebody wants to dig through all of > these files by Friday. This code is quite crusty so I don't think leaving it > alone would be that harmful. It seems better than letting these macros get > pushed around like that. I am OK with disabling clang-format on these directories, particularly if we can comment as to why the directories at getting ignored in the file that disables them.

Sounds like a plan. I'll file a followup bug to use clang-format off because it would be nice for the rest of each file to have a nicer format.

Sounds great!

Many of the inline assembly macros in these files get reformatted poorly by clang-format, so don't format them for now. In followup work, in bug 1510781, hopefully we can write a larger patch to only disable clang-format for the specific macros and then format the rest of the files. The problematic include STUB_ENTRY and STUB_MANGLED_ENTRY.

This patch disables clang formatting for the two XPTCall stubs directories. I verified locally that when running ./mach clang-format -p xpcom/reflect/xptcall/ that these two directories are not formatted.

Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c67e1875a652 Disable clang-formatting for XPTCall stub files. r=Ehsan

Comment on attachment 9028458 [details] Bug 1510478 - Disable clang-formatting for XPTCall stub files. [ESR Uplift Approval Request] If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is required for easier backporting of patches after the reformatting of ESR using clang-format. User impact if declined: Declining this will negatively impact our developers' ability to easily backport their patches to ESR in the future. Fix Landed on Version: 65 Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): This is NPOTB. String or UUID changes made by this patch: None

Comment on attachment 9028458 [details] Bug 1510478 - Disable clang-formatting for XPTCall stub files. OK for uplift to ESR for clang-format project.