aaron.ballman added a comment. In D104601#2886412 <https://reviews.llvm.org/D104601#2886412>, @Meinersbur wrote:
> In D104601#2877855 <https://reviews.llvm.org/D104601#2877855>, @aaron.ballman > wrote: > >> I don't think they'd *add* it, I worry it's already used by their build >> system for reasons unknown and the developer replicates it when reporting >> the bug because they're not looking at what flags can be removed. > > I made the flag an error if used without `-E`, so the build system cannot add > it. > > Flags such as `-P`, `-fuse-line-directives`, `-frewrite-imports` and > `-frewrite-includes` are also silently ignored without `-E`. This requirement > seems exclusive for `-felide-unnecessary-whitespace`. It also suggests that > it is not a problematic scenario. Perhaps it's not a problematic scenario, but I'd consider those cases to also be bugs. Silently ignoring things the user wrote (either in code or on the command line) is typically user-hostile. >>>> The "very long line" example mentioned by @dblaikie is sort of along these >>>> lines (sorry for the bad pun). >>> >>> I can add forced line breaks (after/before 80 cols? configurable?) if >>> requested. >> >> I don't think that's necessary just yet. If there's an issue in practice, it >> seems like we could address it once we have the real world use in front of >> us. However, It'd help my confidence if you were able to run this >> functionality over a large corpus of code to see if any issues do pop out, >> if that's plausible for you. > > I ran it (using ccache) over llvm-project and the llvm-test-suite. Is there > another large corpus do you have in mind? A Linux or BSD distro worth of code would be really nice, but I don't insist. The trouble with llvm-project is that it has a somewhat uniform style for code, and I've found that tools in distro packages tend to exercise more interesting cases sometimes. >> I also think we should rename it because "normalize whitespace" can be >> ambiguous depending on what context you're reading the words in. > > Changed to your suggested `-felide-unnecessary-whitespace`, although I think > name will not allow use to insert whitespace e.g. for keeping the line length > down. If that's not required, I would prefer `-fminimize-whitespace` over > -felide-unnecessary-whitespace`. I'd be perfectly happy with `-fminimize-whitespace` as well if that's your preference. ================ Comment at: clang/docs/ClangCommandLineReference.rst:2484 +-P option to normlize whitespace such that two files with only formatted +changes are equal. + ---------------- Meinersbur wrote: > aaron.ballman wrote: > > Should this option be ignored when not performing a preprocessing action > > (and documented as such)? > I changed it to an error if used without `-E`. Thanks! Can you add that info to the documentation here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
