This fixes false positive cases where a reference is initialized outside of a
block statement and then its initializing variable is modified. Another case is
when the looped over container is modified.
Differential Revision: https://reviews.llvm.org/D103021
Reviewed-by: ymandel
Ignored diagnostics were only checked after level adjusters and assumed
it would stay the same for the rest. But it can also be modified by
FeatureModules.
Differential Revision: https://reviews.llvm.org/D103387
Given `int foo, bar;`, TraverseAST reveals this tree:
TranslationUnitDecl
- foo
- bar
Before this patch, with the TraversalScope set to {foo}, TraverseAST yields:
foo
After this patch it yields:
TranslationUnitDecl
- foo
Also, TraverseDecl(TranslationUnitDecl) now respects the traversal scope.
---
The main effect of this today is that clang-tidy checks that match the
translationUnitDecl(), either in order to traverse it or check
parentage, should work.
Differential Revision: https://reviews.llvm.org/D104071
<string> is currently the highest impact header in a clang+llvm build:
https://commondatastorage.googleapis.com/chromium-browser-clang/llvm-include-analysis.html
One of the most common places this is being included is the APInt.h header, which needs it for an old toString() implementation that returns std::string - an inefficient method compared to the SmallString versions that it actually wraps.
This patch replaces these APInt/APSInt methods with a pair of llvm::toString() helpers inside StringExtras.h, adjusts users accordingly and removes the <string> from APInt.h - I was hoping that more of these users could be converted to use the SmallString methods, but it appears that most end up creating a std::string anyhow. I avoided trying to use the raw_ostream << operators as well as I didn't want to lose having the integer radix explicit in the code.
Differential Revision: https://reviews.llvm.org/D103888
mixed integer and floating point types with WarnOnEquivalentBitWidth=0.
Also standardize control flow of handleX conversion functions to make it easier to be consistent.
Patch by Stephen Concannon!
Differential Revision: https://reviews.llvm.org/D103894
There is a followup fix for a unit test introduced at D102906. The test file was placed into a temp folder and test assumed that it would be visible without the full path specification.
This behaviour can be changed in future and it would be good to specify full path to the file at the test.
Test Plan:
```
ninja check-clang-tools
```
Reviewed By: DmitryPolukhin
Differential Revision: https://reviews.llvm.org/D104021
This fixes a false positive case where for instance a pointer is obtained and declared using `auto`.
Differential Revision: https://reviews.llvm.org/D103018
Reviewed-by: ymandel
It is not useful to keep the statement around and can lead to compiler
warnings when -Wall (-Wunused-variable specifically) turned on.
Differential Revision: https://reviews.llvm.org/D102175
Reviewed-by: ymandel
This renames the expression value categories from rvalue to prvalue,
keeping nomenclature consistent with C++11 onwards.
C++ has the most complicated taxonomy here, and every other language
only uses a subset of it, so it's less confusing to use the C++ names
consistently, and mentally remap to the C names when working on that
context (prvalue -> rvalue, no xvalues, etc).
Renames:
* VK_RValue -> VK_PRValue
* Expr::isRValue -> Expr::isPRValue
* SK_QualificationConversionRValue -> SK_QualificationConversionPRValue
* JSON AST Dumper Expression nodes value category: "rvalue" -> "prvalue"
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D103720
This implements the 'using enum maybe-qualified-enum-tag ;' part of
1099. It introduces a new 'UsingEnumDecl', subclassed from
'BaseUsingDecl'. Much of the diff is the boilerplate needed to get the
new class set up.
There is one case where we accept ill-formed, but I believe this is
merely an extended case of an existing bug, so consider it
orthogonal. AFAICT in class-scope the c++20 rule is that no 2 using
decls can bring in the same target decl ([namespace.udecl]/8). But we
already accept:
struct A { enum { a }; };
struct B : A { using A::a; };
struct C : B { using A::a;
using B::a; }; // same enumerator
this patch permits mixtures of 'using enum Bob;' and 'using Bob::member;' in the same way.
Differential Revision: https://reviews.llvm.org/D102241
This is a pre-patch for adding using-enum support. It breaks out
the shadow decl handling of UsingDecl to a new intermediate base
class, BaseUsingDecl, altering the decl hierarchy to
def BaseUsing : DeclNode<Named, "", 1>;
def Using : DeclNode<BaseUsing>;
def UsingPack : DeclNode<Named>;
def UsingShadow : DeclNode<Named>;
def ConstructorUsingShadow : DeclNode<UsingShadow>;
Differential Revision: https://reviews.llvm.org/D101777
TestTU now prints errors to llvm::errs and aborts on failures via
llvm_unreachable, rather than executing ASSERT_FALSE.
We'd like to make use of these testing libraries in different test suits that
might be compiling with a different gtest version than LLVM has.
Differential Revision: https://reviews.llvm.org/D103685
In --check mode we do not run code completion because it is too slow,
especially on larger files. With the introducation of --check-lines we
can narrow down the scope and thus we can afford to do code completion.
We vlog() the top completion result, but that's not really the point.
The most value will come from being able to reproduce crashes that occur
during code completion and require preamble build or index (and thus are
more difficult to reproduce with -code-complete-at).
Differential Revision: https://reviews.llvm.org/D103538
This is causing weird code patterns in various places and I can't see
any difference between None and empty change list. Neither in the current use
cases nor in the spec.
Differential Revision: https://reviews.llvm.org/D103449
This enables requests like workspaceSymbols to be dispatched using the
file user was most recently operating on. A replacement for D103179.
Differential Revision: https://reviews.llvm.org/D103476
This allows us to differentiate symbols from the system (e.g. system
includes or sysroot) differently than symbols defined in the user's
project, which can be used by editors to display them differently.
This is currently based on `FileCharacteristic`, but we can
consider alternatives such as `Sysroot` and file paths in the future.
Differential Revision: https://reviews.llvm.org/D101554
We now make up a TypeLoc for the class receiver to simplify visiting,
notably for indexing, availability, and clangd.
Differential Revision: https://reviews.llvm.org/D101645
When completing an Objective-C method declaration by name, we need to
preserve the leading text as a `qualifier` so we insert it properly
before the first typed text chunk.
Differential Revision: https://reviews.llvm.org/D100798
This patch starts to produce a very obvious false-positives,
despite the fact the preexisting tests already cover the pattern.
they clearly don't actually cover it.
https://godbolt.org/z/3zdqvbfxj
This reverts commit 1709bb8c73.
Previously notification of the Server about semantic happened strictly
before notification of the AST thread.
Hence a racy Server could make a request (like semantic tokens) after
the notification, with the assumption that it'll be served fresh
content. But it wasn't true if AST thread wasn't notified about the
change yet.
This change reverses the order of those notifications to prevent racy
interactions.
Differential Revision: https://reviews.llvm.org/D102761
The diff adds Remark to Diagnostic::Level for clang tooling. That makes
Remark diagnostic level ready to use in clang-tidy checks: the
clang-diagnostic-module-import becomes visible as a part of the change.
getSourceText could return an empty string for error cases (e.g. invalid
source locaiton), this patch makes the code more robust.
The crash did happen in our internal codebase, but unfortunately I
didn't manage to get a reproduce case. One thing I can confirm from
the core dump is that the crash is caused by calling isRawStringLiteral
on an empty Text.
Differential Revision: https://reviews.llvm.org/D102770
We can only use ASTContext::getTypeInfo for complete types.
This fixes bugzilla issue 50313.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D102569
Execute implementations already checks for permissions and existence
and returns relevant errors as necessary, so instead of printing our own errors,
we just print theirs.
This also fixes a case in windows where the driver might be missing the `.exe`
suffix. Previously, clangd would reject such a driver because sys::fs::exists is
strict, whereas the underlying Execute implementation would check with `.exe`
suffix too.
Fixes https://github.com/clangd/clangd/issues/93
Differential Revision: https://reviews.llvm.org/D102431
Tweaks like DefineOutline depend on FS to be set at `apply()` time.
After https://reviews.llvm.org/D93978, tweaks run from Check tool lost
access to FS. This makes the available to apply() once again.
Differential Revision: https://reviews.llvm.org/D102519
Clang would emit a fatal error when it encounters an unregistered PCH
format. This change ensures clangd will always use raw format no matter what
user specifies.
As side effects:
- serializing an AST in an unknown format might throw off build
systems. I suppose this would only be an issue when build system and clangd are
racing for same PCM modules, hopefully this should be rare and both clangd or
the build system should recover on the next run.
- whenever clang reads a serialized AST it seems to be checking for file
signature and emitting non-fatal errors. so this should be fine again.
The only other valid module format in clang is `obj` but it is part of codegen,
i don't think it is worth the dependency. Hence chosing to not register it, at
least yet.
Differential Revision: https://reviews.llvm.org/D102418
If the loop condition is a value of an instance variable, a property value,
or a message result value, it's a good indication that the loop is not infinite
and we have a really hard time proving the opposite so suppress the warning.
Differential Revision: https://reviews.llvm.org/D102294
Take advantage of the new ASTMatcher added in D102213 to fix massive false negatives of the infinite loop checker on Objective-C.
Differential Revision: https://reviews.llvm.org/D102214
This commit fixes the cppcoreguidelines-pro-type-vararg test when it
runs on a Windows host, but the toolchain is targeted a non-Windows
platform.
Reviewed By: njames93
Differential Revision: https://reviews.llvm.org/D102337
Non-comprehensive list of cases:
* Dumping template arguments;
* Corresponding parameter contains a deduced type;
* Template arguments are for a DeclRefExpr that hadMultipleCandidates()
Type information is added in the form of prefixes (u8, u, U, L),
suffixes (U, L, UL, LL, ULL) or explicit casts to printed integral template
argument, if MSVC codeview mode is disabled.
Differential revision: https://reviews.llvm.org/D77598
Within clang-tidy's NarrowingConversionsCheck.
* Allow opt-out of some common occurring patterns, such as:
- Implicit casts between types of equivalent bit widths.
- Implicit casts occurring from the return of a ::size() method.
- Implicit casts on size_type and difference_type.
* Allow opt-in of errors within template instantiations.
This will help projects adopt these guidelines iteratively.
Developed in conjunction with Yitzhak Mandelbaum (ymandel).
Patch by Stephen Concannon!
Differential Revision: https://reviews.llvm.org/D99543
There should be a follow up to this for changing the traversal mode, but some of the tests don't like that.
Reviewed By: steveire
Differential Revision: https://reviews.llvm.org/D101614
Currently client was setting the HasMore to true iff stream said so.
Hence if we had a broken stream for whatever reason (e.g. hitting deadline for a
huge response), HasMore would be false, which is semantically incorrect (e.g.
will throw rename off).
Differential Revision: https://reviews.llvm.org/D101915
This is to prevent server from being DOS'd by possible malicious
parties issuing requests that can yield huge responses.
One possible drawback is on rename workflow. As it really requests all
occurences, but it has an internal limit on 50 files currently.
We are putting the limit on 10000 elements per response So for rename to regress
one should have 10k refs to a symbol in less than 50 files. This seems unlikely
and we fix it if there are complaints by giving up on the response based on the
number of files covered instead.
Differential Revision: https://reviews.llvm.org/D101914
When a variable is used in an initializer of an aggregate
for its reference-type field this counts as aliasing.
Differential Revision: https://reviews.llvm.org/D101791
D96215 takes care of the situation where the variable is captured into
a nearby lambda. This patch takes care of the situation where
the current function is the lambda and the variable is one of its captures
from an enclosing scope.
The analogous problem for ^{blocks} is already handled automagically
by D96215.
Differential Revision: https://reviews.llvm.org/D101787
The utility function clang::tidy::utils::hasPtrOrReferenceInFunc() scans the
function for pointer/reference aliases to a given variable. It currently scans
for operator & over that variable and for declarations of references to that
variable.
This patch makes it also scan for C++ lambda captures by reference
and for Objective-C block captures.
Differential Revision: https://reviews.llvm.org/D96215
This lint check is a part of the FLOCL (FPGA Linters for OpenCL) project
out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera ID dependent backward branch lint check finds ID dependent
variables and fields used within loops, and warns of their usage. Using
these variables in loops can lead to performance degradation.
Treat them just like we do for properties - as a `property` semantic
token although ideally we could differentiate the two.
Differential Revision: https://reviews.llvm.org/D101785
Having nested macros in the C code could cause clangd to fail an assert in clang::Preprocessor::setLoadedMacroDirective() and crash.
#1 0x00000000007ace30 PrintStackTraceSignalHandler(void*) /qdelacru/llvm-project/llvm/lib/Support/Unix/Signals.inc:632:1
#2 0x00000000007aaded llvm::sys::RunSignalHandlers() /qdelacru/llvm-project/llvm/lib/Support/Signals.cpp:76:20
#3 0x00000000007ac7c1 SignalHandler(int) /qdelacru/llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1
#4 0x00007f096604db20 __restore_rt (/lib64/libpthread.so.0+0x12b20)
#5 0x00007f0964b307ff raise (/lib64/libc.so.6+0x377ff)
#6 0x00007f0964b1ac35 abort (/lib64/libc.so.6+0x21c35)
#7 0x00007f0964b1ab09 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21b09)
#8 0x00007f0964b28de6 (/lib64/libc.so.6+0x2fde6)
#9 0x0000000001004d1a clang::Preprocessor::setLoadedMacroDirective(clang::IdentifierInfo*, clang::MacroDirective*, clang::MacroDirective*) /qdelacru/llvm-project/clang/lib/Lex/PPMacroExpansion.cpp:116:5
An example of the code that causes the assert failure:
```
...
```
During code completion in clangd, the macros will be loaded in loadMainFilePreambleMacros() by iterating over the macro names and calling PreambleIdentifiers->get(). Since these macro names are store in a StringSet (has StringMap underlying container), the order of the iterator is not guaranteed to be same as the order seen in the source code.
When clangd is trying to resolve nested macros it sometimes attempts to load them out of order which causes a macro to be stored twice. In the example above, ECHO2 macro gets resolved first, but since it uses another macro that has not been resolved it will try to resolve/store that as well. Now there are two MacroDirectives stored in the Preprocessor, ECHO and ECHO2. When clangd tries to load the next macro, ECHO, the preprocessor fails an assert in clang::Preprocessor::setLoadedMacroDirective() because there is already a MacroDirective stored for that macro name.
In this diff, I check if the macro is already inside the IdentifierTable and if it is skip it so that it is not resolved twice.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D101870
The current code accounts for two possible layouts, but there is at
least a third supported layout: clang-tools-extra may also be checked
out as clang/tools/extra with the releases, which was not yet handled.
Rather than treating that as a special case, use the location of
CompletionModel.cmake to handle all three cases. This should address the
problems that prompted D96787 and the problems that prompted the
proposed revert D100625.
Reviewed By: usaxena95
Differential Revision: https://reviews.llvm.org/D101851
Change instances where options which are boolean are assigned the value 1|0 to use true|false instead.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101721
This commit fixes cppcoreguidelines-pro-type-vararg false positives on
'char *' variables.
The incorrect warnings generated by clang-tidy can be illustrated with
the following minimal example:
```
goid foo(char* in) {
char *tmp = in;
}
```
The problem is that __builtin_ms_va_list desugared as 'char *', which
leads to false positives.
Fixes bugzilla issue 48042.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101259
Due to a somewhat annoying, but necessary, shortfall in -Wunused-lambda-capture, These unused captures aren't warned about.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D101611
Class properties are always implicit short-hands for the getter/setter
class methods.
We need to explicitly visit the interface decl `UIColor` in `UIColor.blueColor`,
otherwise we instead show the method decl even while hovering over
`UIColor` in the expression.
Differential Revision: https://reviews.llvm.org/D99975
This is useful for running in batch mode.
Getting the SymbolID from via getSymbolInfo may give SymbolID
of a symbol different from that located by LocateSymbolAt (they
have different semantics of choosing the symbol.)
Differential Revision: https://reviews.llvm.org/D101388
Checks if introspection support is available set output kind parser.
If it isn't present the auto complete will not suggest `srcloc` and an error query will be reported if a user tries to access it.
Reviewed By: steveire
Differential Revision: https://reviews.llvm.org/D101365
This is fix for some timeouts and OOM problems faced while indexing an
auto-generated file with thousands of nested lambdas.
Differential Revision: https://reviews.llvm.org/D101066
If no range is given, return the translation unit AST.
This is useful for tooling operations that require e.g. the full path to
a node.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D101057
Mishandling of variadic arguments in a function call caused a crash
(runtime assert fail) in bugprone-infinite-loop tidy checker. Fix
is to limit argument matching to the lesser of the number of variadic
params in the prototype or the number of actual args in the call.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D101108
When building preamble, clangd truncates file contents. This yielded
errnous warnings in some cases.
This patch fixes the issue by turning off no-newline-at-eof warnings whenever
the file has more contents than the preamble.
Fixes https://github.com/clangd/clangd/issues/744.
Differential Revision: https://reviews.llvm.org/D100501
clang-tidy should not generate warnings for the goto argument without
parentheses, because it would be a syntax error.
The only valid case where an argument can be enclosed in parentheses is
"Labels as Values" gcc extension: https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html.
This commit adds support for the label-as-values extension as implemented in clang.
Fixes bugzilla issue 49634.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D99924
Cross file tweaks can now use the dirty buffer contents easily when performing cross file effects.
This can be noted on the DefineOutline tweak, now working when the target file is unsaved
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D93978
Overflows are never fun.
In most cases (in most of the code), they are rare,
because usually you e.g. don't have as many elements.
However, it's exceptionally easy to fall into this pitfail
in code that deals with images, because, assuming 4-channel 32-bit FP data,
you need *just* ~269 megapixel image to case an overflow
when computing at least the total byte count.
In [[ https://github.com/darktable-org/darktable | darktable ]], there is a *long*, painful history of dealing with such bugs:
* https://github.com/darktable-org/darktable/pull/7740
* https://github.com/darktable-org/darktable/pull/7419
* eea1989f2c
* 70626dd95b
* https://github.com/darktable-org/darktable/pull/670
* 38c69fb1b2
and yet they clearly keep resurfacing still.
It would be immensely helpful to have a diagnostic for those patterns,
which is what this change proposes.
Currently, i only diagnose the most obvious case, where multiplication
is directly widened with no other expressions inbetween,
(i.e. `long r = (int)a * (int)b` but not even e.g. `long r = ((int)a * (int)b)`)
however that might be worth relaxing later.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D93822
These can be invoked at different stages while building an AST to let
FeatureModules implement features on top of it. The patch also
introduces a sawDiagnostic hook, which can mutate the final clangd::Diag
while reading a clang::Diagnostic.
Differential Revision: https://reviews.llvm.org/D98499
First patch to enable diagnostic fix generation through modules. The
workflow will look like:
- ASTWorker letting modules know about diagnostics while building AST,
modules can read clang::Diagnostic and mutate clangd::Diagnostic through
that hook.
- Modules can implement and expose tweaks to fix diagnostics or act as
general refactorings.
- Tweak::Selection will contain information about the diagnostic
associated with the codeAction request to enable modules to fail their
diagnostic fixing tweakson prepare if need be.
Differential Revision: https://reviews.llvm.org/D98498
This is the only remaining check that creates `std::move` includes but doesn't add a `<utility>` include.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D97683
This reverts commit 63bc9e4435.
This breaks llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:570:11:
with error: enumeration value 'None' not handled in switch [-Werror,-Wswitch]
(this was originally part of https://reviews.llvm.org/D96281 and has been split off into its own patch)
If a macro is used within a function, the code inside the macro
doesn't make the code less readable. Instead, for a reader a macro is
more like a function that is called. Thus the code inside a macro
shouldn't increase the complexity of the function in which it is called.
Thus the flag 'IgnoreMacros' is added. If set to 'true' code inside
macros isn't considered during analysis.
This isn't perfect, as now the code of a macro isn't considered at all,
even if it has a high cognitive complexity itself. It might be better if
a macro is considered in the analysis like a function and gets its own
cognitive complexity. Implementing such an analysis seems to be very
complex (if possible at all with the given AST), so we give the user the
option to either ignore macros completely or to let the expanded code
count to the calling function's complexity.
See the code example from vgeof (originally added as note in https://reviews.llvm.org/D96281)
bool doStuff(myClass* objectPtr){
if(objectPtr == nullptr){
LOG_WARNING("empty object");
return false;
}
if(objectPtr->getAttribute() == nullptr){
LOG_WARNING("empty object");
return false;
}
use(objectPtr->getAttribute());
}
The LOG_WARNING macro itself might have a high complexity, but it do not make the
the function more complex to understand like e.g. a 'printf'.
By default 'IgnoreMacros' is set to 'false', which is the original behavior of the check.
Reviewed By: lebedev.ri, alexfh
Differential Revision: https://reviews.llvm.org/D98070
Users can reset any external index set by previous fragments by
putting a `None` for the external block, e.g:
```
Index:
External: None
```
Differential Revision: https://reviews.llvm.org/D100106
Without the fix we get:
06:31:09 In file included from ../../clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:3:
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1392:11: error: comparison of integers of different signs: 'const int' and 'const unsigned int' [-Werror,-Wsign-compare]
06:31:09 if (lhs == rhs) {
06:31:09 ~~~ ^ ~~~
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1421:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned int>' requested here
06:31:09 return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
06:31:09 ^
06:31:09 ../../clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:60:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<int, unsigned int>' requested here
06:31:09 EXPECT_EQ(4, Errors[0].Message.FileOffset);
06:31:09 ^
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1924:63: note: expanded from macro 'EXPECT_EQ'
06:31:09 EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
06:31:09 ^
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1392:11: error: comparison of integers of different signs: 'const int' and 'const unsigned long' [-Werror,-Wsign-compare]
06:31:09 if (lhs == rhs) {
06:31:09 ~~~ ^ ~~~
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1421:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned long>' requested here
06:31:09 return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
06:31:09 ^
06:31:09 ../../clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:64:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<int, unsigned long>' requested here
06:31:09 EXPECT_EQ(1, Errors[0].Message.Ranges.size());
06:31:09 ^
06:31:09 ../utils/unittest/googletest/include/gtest/gtest.h:1924:63: note: expanded from macro 'EXPECT_EQ'
06:31:09 EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
06:31:09 ^
06:31:09 2 errors generated.
There was an off-by-one issue with calculating the *exact* end location
of token ranges (as given by SomeDecl->getSourceRange()) which resulted in:
xxx(something)
^~~~~~~~ // Note the missing ~ under the last character.
In addition, a test is added to keep the behaviour in check in the future.
This patch hotfixes commit 3b677b81ce.
Fixes bug http://bugs.llvm.org/show_bug.cgi?id=49000.
This patch allows Clang-Tidy checks to do
diag(X->getLocation(), "text") << Y->getSourceRange();
and get the highlight of `Y` as expected:
warning: text [blah-blah]
xxx(something)
^ ~~~~~~~~~
Reviewed-By: aaron.ballman, njames93
Differential Revision: http://reviews.llvm.org/D98635
Before this change clangd would emit a diagnostic whenever remote-index
was configured but binary didn't have grpc support.
This can be annoying when projects are configuring remote-index through their
configs but developers have a clangd binary without the support.
Differential Revision: https://reviews.llvm.org/D100103
This will allow us to add code completion, which is too expensive at
every token, to --check too.
Differential Revision: https://reviews.llvm.org/D98970
When you pass in a payload with an invalid URI in a build with assertions enabled, it will crash.
Consuming the error from the failed URI parse prevents the error.
The crash is caused by the [llvm::expected](https://llvm.org/doxygen/classllvm_1_1Expected.html) having protection around trying to deconstruct without consuming the error first.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D99872
The default setting for CheckImplicitCasts was changed in
https://reviews.llvm.org/D32164 but the documentation was not updated.
This simple change just syncs the documentation with the behavior of
that checker.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D99991
This allows users to be more precise and exclude a type in a specific namespace
from triggering the check instead of excluding all types with the same
unqualified name.
This change should not interfere with correctly configured clang-tidy setups
since an AllowedType with "::" would never match.
Differential Revision: https://reviews.llvm.org/D98738
Reviewed-by: ymandel, hokein
As proposed in D97109, I tried to make target creation consistent in `clang` and `clangd` by replacing the original procedure with a single function introduced in D97493.
This also helps `clangd` works with CUDA, OpenMP, etc.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D98128
Problem:
On SystemZ we need to open text files in text mode. On Windows, files opened in text mode adds a CRLF '\r\n' which may not be desirable.
Solution:
This patch adds two new flags
- OF_CRLF which indicates that CRLF translation is used.
- OF_TextWithCRLF = OF_Text | OF_CRLF indicates that the file is text and uses CRLF translation.
Developers should now use either the OF_Text or OF_TextWithCRLF for text files and OF_None for binary files. If the developer doesn't want carriage returns on Windows, they should use OF_Text, if they do want carriage returns on Windows, they should use OF_TextWithCRLF.
So this is the behaviour per platform with my patch:
z/OS:
OF_None: open in binary mode
OF_Text : open in text mode
OF_TextWithCRLF: open in text mode
Windows:
OF_None: open file with no carriage return
OF_Text: open file with no carriage return
OF_TextWithCRLF: open file with carriage return
The Major change is in llvm/lib/Support/Windows/Path.inc to only set text mode if the OF_CRLF is set.
```
if (Flags & OF_CRLF)
CrtOpenFlags |= _O_TEXT;
```
These following files are the ones that still use OF_Text which I left unchanged. I modified all these except raw_ostream.cpp in recent patches so I know these were previously in Binary mode on Windows.
./llvm/lib/Support/raw_ostream.cpp
./llvm/lib/TableGen/Main.cpp
./llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
./llvm/unittests/Support/Path.cpp
./clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
./clang/lib/Frontend/CompilerInstance.cpp
./clang/lib/Driver/Driver.cpp
./clang/lib/Driver/ToolChains/Clang.cpp
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D99426
Clangd drops symbols from static index whenever the dynamic index is
authoritative for the file. This results in regressions when static and
dynamic index contains different set of information, e.g.
IncludeHeaders.
After this patch, we'll choose to merge symbols from static index with
dynamic one rather than just dropping. This implies correctness problems
when the definition/documentation of the symbol is deleted. But seems
like it is worth having in more cases.
We still drop symbols if dynamic index owns the file and didn't report
the symbol, which means symbol is deleted.
Differential Revision: https://reviews.llvm.org/D98538
`expandedTokens(SourceRange)` used to do a binary search to get the
expanded tokens belonging to a source range. Each binary search uses
`isBeforeInTranslationUnit` to order two source locations. This is
inherently very slow.
By profiling clangd we found out that users like clangd::SelectionTree
spend 95% of time in `isBeforeInTranslationUnit`. Also it is worth
noting that users of `expandedTokens(SourceRange)` majorly use ranges
provided by AST to query this funciton. The ranges provided by AST are
token ranges (starting at the beginning of a token and ending at the
beginning of another token).
Therefore we can avoid the binary search in majority of the cases by
maintaining an index of ExpandedToken by their SourceLocations. We still
do binary search for ranges which are not token ranges but such
instances are quite low.
Performance:
`~/build/bin/clangd --check=clang/lib/Serialization/ASTReader.cpp`
Before: Took 2:10s to complete.
Now: Took 1:13s to complete.
Differential Revision: https://reviews.llvm.org/D99086
Both the mpi-type-mismatch and mpi-buffer-deref check make use of a static MPIFunctionClassifier object.
This causes issue as the classifier is initialized with the first ASTContext that produces a match.
If the check is enabled on multiple translation units in a single clang-tidy process, this classifier won't be reinitialized for each TU. I'm not an expert in the MPIFunctionClassifier but I'd imagine this is a source of UB.
It is suspected that this bug may result in the crash caused here: https://bugs.llvm.org/show_bug.cgi?id=48985. However even if not the case, this should still be addressed.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98275
This diff patch fixes issue with new line character after check name and before comma. Also ignores all other types of spaces like TAB.
Test Plan: ninja check-clang-tools
Differential Revision: https://reviews.llvm.org/D99180
This lint check is a part of the FLOCL (FPGA Linters for OpenCL)
project out of the Synergy Lab at Virginia Tech.
FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.
The altera unroll loops check finds inner loops that have not been
unrolled, as well as fully-unrolled loops that should be partially
unrolled due to unknown loop bounds or a large number of loop
iterations.
Based on the Altera SDK for OpenCL: Best Practices Guide.
Dummy is a word with inappropriate associations. This patch updates the
references to it in clangd code base with more precise ones.
The only user-visible change is the default variable name used when extracting a
variable. It will be named as `placeholder` from now on.
Differential Revision: https://reviews.llvm.org/D99065
llvm supports specifying a non-standard layout where each project lies in its
own place. Do not assume a fixed layout and use the appropriate cmake variable
instead.
Differential Revision: https://reviews.llvm.org/D96787
Don't emit a warning if the `continue` appears in a switch context as changing it to `break` will break out of the switch rather than a do loop containing the switch.
Fixes https://llvm.org/PR49492.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98338
The idiom:
```
DeclContext::lookup_result R = DeclContext::lookup(Name);
for (auto *D : R) {...}
```
is not safe when in the loop body we trigger deserialization from an AST file.
The deserialization can insert new declarations in the StoredDeclsList whose
underlying type is a vector. When the vector decides to reallocate its storage
the pointer we hold becomes invalid.
This patch replaces a SmallVector with an singly-linked list. The current
approach stores a SmallVector<NamedDecl*, 4> which is around 8 pointers.
The linked list is 3, 5, or 7. We do better in terms of memory usage for small
cases (and worse in terms of locality -- the linked list entries won't be near
each other, but will be near their corresponding declarations, and we were going
to fetch those memory pages anyway). For larger cases: the vector uses a
doubling strategy for reallocation, so will generally be between half-full and
full. Let's say it's 75% full on average, so there's N * 4/3 + 4 pointers' worth
of space allocated currently and will be 2N pointers with the linked list. So we
break even when there are N=6 entries and slightly lose in terms of memory usage
after that. We suspect that's still a win on average.
Thanks to @rsmith!
Differential revision: https://reviews.llvm.org/D91524
The deprecation notice was cherrypicked to the release branch in f8b3298924 so its safe to remove this for the 13.X release cycle.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98612
This allows requesting information about the server uptime and start time. This is the first patch in a series of monitoring changes, hence it's not immediately useful. Next step is propagating the index freshness information and then probably loading metadata into the index server.
The way to test new behaviour through command line:
```
$ grpc_cli call localhost:50051 Monitor/MonitoringInfo ''
connecting to localhost:50051
uptime_seconds: 42
index_age_seconds: 609568
Rpc succeeded with OK status
```
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D98246
Implement initial support for pull-based diagnostics in ClangdServer.
This is planned for LSP 3.17, and initial proposal is in
d15eb0671e/protocol/src/common/proposed.diagnostic.ts (L111).
We chose to serve the requests only when clangd has a fresh preamble
available. In case of a stale preamble we just drop the request on the
floor.
This patch doesn't plumb this to LSP layer yet, as pullDiags is still a
proposal with only an implementation in vscode.
Differential Revision: https://reviews.llvm.org/D98623
Somewhat surprisingly, signature help is emitted as a side-effect of
computing the expected type of a function argument.
The reason is that both actions require enumerating the possible
function signatures and running partial overload resolution, and doing
this twice would be wasteful and complicated.
Change #1: document this, it's subtle :-)
However, sometimes we need to compute the expected type without having
reached the code completion cursor yet - in particular to allow
completion of designators.
eb4ab3358c did this but introduced a
regression - it emits signature help in the wrong location as a side-effect.
Change #2: only emit signature help if the code completion cursor was reached.
Currently there is PP.isCodeCompletionReached(), but we can't use it
because it's set *after* running code completion.
It'd be nice to set this implicitly when the completion token is lexed,
but ConsumeCodeCompletionToken() makes this complicated.
Change #3: call cutOffParsing() *first* when seeing a completion token.
After this, the fact that the Sema::Produce*SignatureHelp() functions
are even more confusing, as they only sometimes do that.
I don't want to rename them in this patch as it's another large
mechanical change, but we should soon.
Change #4: prepare to rename ProduceSignatureHelp() to GuessArgumentType() etc.
Differential Revision: https://reviews.llvm.org/D98488
For some reason the initial implementation of the check had an explicit check
for the main file to avoid being applied in headers. This diff removes this
check and add a test for the check on a header.
Similar approach was proposed in D61989 but review there got stuck.
Test Plan: added new test case
Differential Revision: https://reviews.llvm.org/D97563
If a identifier has a correct prefix/suffix but a bad case, the fix won't strip them when computing the correct case, leading to duplication when the are added back.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98521
This enables unifying command line flags with config options in clangd
internals. This patch changes behaviour in 2 places:
- BackgroundIndex was previously disabled when -remote-index was
provided. After this patch, it will be enabled but all files will have
bkgindex policy set to Skip.
- -index-file was loaded at startup (at least load was initiated), now
the load will happen through ProjectAwareIndex with first index query.
Unfortunately this doesn't simplify any options initially, as
- CompileCommandsDir is also used by clangd --check workflow, which
doesn't use configs.
- EnableBackgroundIndex option controls whether the component will be
created at all, which implies creation of extra threads registering a
listener for compilation database discoveries.
Differential Revision: https://reviews.llvm.org/D98029
Explicit specifier can only be mentioned on the in-line declaration of a
constructor, so don't carry it over to the definition.
Differential Revision: https://reviews.llvm.org/D98164
Also give CanonicalIncludes a less powerful interface (canonicalizes
symbols vs headers separately) so we can cache its results better.
Prior to this:
- path->uri conversions were not consistently cached, this is
particularly cheap when we start from a FileEntry* (which we often can)
- only a small fraction of header-to-include calculation was cached
This is a significant speedup at least for dynamic indexing of preambles.
On my machine, opening XRefs.cpp:
```
PreambleCallback 1.208 -> 1.019 (-15.7%)
BuildPreamble 5.538 -> 5.214 (-5.8%)
```
Differential Revision: https://reviews.llvm.org/D98371
Refactor cross file rename to use a Filesystem instead of a function for getting buffer contents of open files.
Depends on D94554
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D95043
This allows sending requests through CLI and more debugging
opportunities. Example:
```bash
$ grpc_cli ls localhost:50051
clang.clangd.remote.v1.SymbolIndex
grpc.reflection.v1alpha.ServerReflection
grpc.health.v1.Health
```
Nathan Ridge