llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.84k stars 11.01k forks source link

Clang-tidy asserts on Clang fixes, which were not enabled #33477

Open EugeneZelenko opened 6 years ago

EugeneZelenko commented 6 years ago
Bugzilla Link 34130
Version unspecified
OS Linux

Extended Description

I experienced Clang-tidy crashes on my work code base:

Fix conflicts with existing fix! The new insertion has the same insert location as an existing replacement. New replacement: ./../obj/linux-x86/debug/Hash.h: 4912:+0:"break; " Existing replacement: ./../obj/linux-x86/debug/Hash.h: 4912:+0:"[[clang::fallthrough]]; " clang-tidy: tools/clang/tools/extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:86: virtual void (anonymous namespace)::ClangTidyDiagnosticRenderer::emitCodeContext(clang::FullSourceLoc, DiagnosticsEngine::Level, SmallVectorImpl &, ArrayRef): Assertion `false && "Fix conflicts with existing fix!"' failed.

But I run only modernize checks, not Clang diagnostics:

-checks="-,modernize-deprecated-headers" -header-filter="." -checks="-,modernize-use-default-member-init" -header-filter="."

Code in question was generated by gperf and contain next switch:

switch (hval) { default: hval += asso_values[static_cast(str[6])]; /FALLTHROUGH/ case 6: hval += asso_values[static_cast(str[5])]; /FALLTHROUGH/ case 5: hval += asso_values[static_cast(str[4])]; /FALLTHROUGH/ case 4: hval += asso_values[static_cast(str[3])]; /FALLTHROUGH/ case 3: hval += asso_values[static_cast(str[2])]; /FALLTHROUGH/ case 2: case 1: hval += asso_values[static_cast(str[0])]; break; }

llvmbot commented 6 years ago

I replaced warning options with -w.

-w is special, since it can't be overridden. Sometimes there's a valid need to enable certain compiler diagnostics via clang-tidy (using -extra-arg=-Wsomething + -checks=clang-diagnostic-something), and then -w would be an obstacle.

Anyway, I think it may be reasonable to disable diagnostics coming from the compilation database. At least I've not seen use cases where it would be wrong.

A step further in that direction might be to make clang diagnostics first-class citizen (i.e. only enable them when requested by the checks filter). In this case we'd need to get the list of diagnostics from Clang, so that we can list them with -list-checks and add appropriate -W flag when requested by the checks filter.

EugeneZelenko commented 6 years ago

I replaced warning options with -w.

llvmbot commented 6 years ago

You mean implicitly adding -Wno-everything (and maybe -Wno-error) to the options received from the compilation database?

EugeneZelenko commented 6 years ago

I also found workaround to disable warnings in compile database. But may be Clang-tidy should not envoke Clang diagnostics unless it explicitly requested in -checks?

llvmbot commented 6 years ago

I suppose the file you're analyzing gets the -Wimplicit-fallthrough compiler argument from a compilation database. The easiest way to work around this is to add -extra-arg=-Wno-implicit-fallthrough (or -Wno-everything for that matter). As for why this diagnostic causes an assertion failure, it's because it has two alternative fixes, which clang-tidy doesn't handle correctly.