hokein marked an inline comment as done.
hokein added a comment.

As discussed offline, the current approach only works for checks provide a 
single fix, providing such API is somehow misleading.

Instead, we'd emit the check fix and the fix description via diagnostic::Note, 
rather than attaching to the main diagnostic of the check:

- match the data model of the checks provide different semantic fixes depending 
on the context;
- align with the way how existing clang diagnostics emit alternative fixes (via 
diagnostic::Note);
- open a door to write clang-tidy checks that provide alternative fixes; we 
don't have these checks at the moment, but some clang diagnostics like 
`clang-diagnostic-parentheses` do (and our current implementation just 
aggregates all the fixes together, which is not correct);

It would require some changes in clang-tidy check side:

Before this patch:

  void MyCheck::check(...) {
     ...
     diag(loc, "my check warning") <<  FixtItHint::CreateReplacement(...);
  }

After this patch:

  void MyCheck::check(...) {
     ...
     diag(loc, "my check warning"); // Emit a check warning
     // We might want to introduce an utility method like `diagFix` to save 
some verbosed words.
     diag(loc, "fix description", DiagnosticIDs::Note) << 
FixtItHint::CreateReplacement(...); // Emit a diagnostic note and a fix
  }

An example of unused-using-decls check (clang-tidy command line output)

- before:

  1 warning generated.
  /tmp/test.cpp:8:12: warning: using decl 'Foo' is unused 
[misc-unused-using-decls]
  using foo::Foo;
  ~~~~~~~~~~~^~~~

- after:

  1 warning generated.
  /tmp/test.cpp:8:12: warning: using decl 'Foo' is unused 
[misc-unused-using-decls]
  using foo::Foo;
             ^
  /tmp/test.cpp:8:12: note: remove the using
  using foo::Foo;
  ~~~~~~~~~~~^~~~


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59932/new/

https://reviews.llvm.org/D59932



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to