JakeMerdichAMD added a comment.
I can see the use of this, but I am also wary that ignoring style options will
lead to people producing different results on different versions of
clang-format. This is both because having set-or-unset an option will naturally
lead to different code and also that newer options are a de-facto check that
clang-format is at least a certain version (we have minor differences between
major versions as bugs are fixed). In any case, I see this being *very* easy to
misuse and the documentation should have a warning reflecting that.
As far as docs go, the bulk are in clang/docs/ClangFormat.rst and require no
additional 'publish' step but definitely should be updated. The command line
options are implicitly generated for the cli tool's --help flag, but the docs
for them are all in that rst file and manually maintained.
================
Comment at: clang/tools/clang-format/ClangFormat.cpp:108
+static cl::opt<bool>
+ IgnoreUnkownOptions("ignore-unknown-options",
+ cl::desc("If set, unknown format options are
ignored."),
----------------
fodinabor wrote:
> MyDeveloperDay wrote:
> > feels like a mouthful is there nothing shorter we could use? -Wignore (or
> > something)
> hmm... `-Wunknown`
> but the `-W` does not really make it clear that the default "errors" should
> now be treated as warnings instead. From compiler conventions, I'd expect the
> `-W` to enable a warning ...
>
> and something like `-Wno-error=unknown` is not really shorter...
I personally like -Wno-error=unknown if the behavior is to emit warnings and
-Wno-unknown if the behavior is to be silent, for consistency with clang/gcc.
I would also put a note in the description saying that ignoring options can
lead to dramatically different output between versions that do and don't
support a given option, so it should be used with care.
================
Comment at: llvm/lib/Support/YAMLTraits.cpp:199
+ if (IgnoreUnkown)
+ return;
for (const auto &NN : MN->Mapping) {
----------------
grimar wrote:
> fodinabor wrote:
> > MyDeveloperDay wrote:
> > > do we want to flat out ignore or just report but not fatally. (just a
> > > thought) silent failures are hard to diagnose
> > true.. don't know what's the best option?
> >
> > keep it as a printed out error and just don't return an error code on exit?
> > This option would make it a clang-format only change, but feels really
> > dirty.
> >
> > Otherwise I'd have to dig my way through to
> > `llvm::yaml::Stream::printError` (or maybe rather add a `printWarning`) to
> > conditionally change the message type for the ignore case.
> Yes, I think we might want to introduce a method, like `Input::reportWarning`
> which will call ` Strm->printError(node, message);`, but will not set a `EC`.
> Also, it should print "warning: ..." instead of "error: ..." prefix somehow.
>
Something like how clang/gcc only report unknown -Wno-foo options if there's
another error is an idea, but clang-format almost never fails unless there's a
bad config or input file so that's not too useful.
I'm fine with either warning or being silent. If the user has opted-into
ignoring missing options, we can assume they're willing to accept the
consequences of such.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
