[PATCH] D113572: [DebugInfo] run clang-format on some unformatted files
Paul Robinson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 10 10:39:32 PST 2021
probinson accepted this revision.
probinson added a comment.
This revision is now accepted and ready to land.
In general doing a clang-format before extensive work on some files is an obvious NFC, but I can see why you might have wanted some review on this.
LGTM, although the one thing I'd really like you to try is reflowing that one awkward function description. All the other trivial spacing stuff is purely optional.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:610
+ DEFINE_MDNODE_GET(DIFile,
+ (MDString * Filename, MDString *Directory,
+ Optional<ChecksumInfo<MDString *>> CS = None,
----------------
It looks like clang-format preserved the spaces around the first asterisk (`MDString * Filename`) but handled the next case correctly (`MDString *Directory`). Can you manually remove the space before Filename and see if clang-format leaves it that way?
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:1780
+ /// encode the raw values provided. \p BD: base discriminator \p DF:
+ /// duplication factor \p CI: copy index The return is None if the values
+ /// cannot be encoded in 32 bits - for example, values for BD or DF larger
----------------
This reflowing is awkward. Try moving each \p to a new line, and maybe indent a couple of spaces, see if that causes clang-format to behave better. You might need an empty `///` line before the \p lines.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2145
+ DEFINE_MDNODE_GET(DILexicalBlock,
+ (DILocalScope * Scope, DIFile *File, unsigned Line,
+ unsigned Column),
----------------
Try removing the space before `Scope` and see if clang-format will preserve that.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2149
+ DEFINE_MDNODE_GET(DILexicalBlock,
+ (Metadata * Scope, Metadata *File, unsigned Line,
+ unsigned Column),
----------------
Try removing the space before `Scope` and see whether clang-format will preserve that.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2196
+ DEFINE_MDNODE_GET(DILexicalBlockFile,
+ (DILocalScope * Scope, DIFile *File,
+ unsigned Discriminator),
----------------
And another `Scope` with an incorrect space.
================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2311
DEFINE_MDNODE_GET(DINamespace,
- (DIScope *Scope, StringRef Name, bool ExportSymbols),
+ (DIScope * Scope, StringRef Name, bool ExportSymbols),
(Scope, Name, ExportSymbols))
----------------
Man, these are everywhere. I'm going to stop pointing out each one.
================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1677
+ /*IsOptimized=*/Record[14], /*Virtuality=*/Record[11],
+ /*DIFlagMainSubprogram*/ HasOldMainSubprogramFlag);
----------------
I think putting an `=` at the end of `DIFlagMainSubprogram` will avoid that space.
================
Comment at: llvm/lib/IR/DIBuilder.cpp:786
File, LineNo, Ty, AlwaysPreserve, Flags,
- /* AlignInBits */0, Annotations);
+ /* AlignInBits */ 0, Annotations);
}
----------------
`/*AlignInBits=*/`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113572/new/
https://reviews.llvm.org/D113572
More information about the llvm-commits
mailing list