The Yaml module is missing on some systems and on many of clang buildbots.
But the test for run-clang-tidy.py doesn't fail due to 'NOT' statement masking a python runtime error.
This patch conditionally imports and enables the yaml module only if it's present in the system.
If not, then '-export-fixes' is disabled.
Differential Revision: https://reviews.llvm.org/D59734
llvm-svn: 357114
Summary:
Still some pieces to go here: unit tests for new SourceCode functionality and
a command-line flag to force utf-8 mode. But wanted to get early feedback.
Reviewers: hokein
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58275
llvm-svn: 357102
Makes the name of this directory consistent with the names of the other
directories in clang-tools-extra.
Similar to r356254. No intended behavior change.
Differential Revision: https://reviews.llvm.org/D59750
llvm-svn: 356897
Summary:
The LSP clients cannot know clangd will not send diagnostic updates
for closed files, so we send them an empty list of diagnostics to
avoid showing stale diagnostics for closed files in the UI, e.g. in the
"Problems" pane of VSCode.
Fixes PR41217.
Reviewers: hokein
Reviewed By: hokein
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59757
llvm-svn: 356880
Use InitLLVM and WithColor
Delete PPTraceConsumer, add the callback in PPTraceAction
Migrae to tooling::createExecutorFromCommandLineArgs
Don't specialize empty OutputFileName
llvm-svn: 356849
Summary:
Finally, we are here!
Analyzes OpenMP Structured Blocks and checks that no exception escapes
out of the Structured Block it was thrown in.
As per the OpenMP specification, structured block is an executable statement,
possibly compound, with a single entry at the top and a single exit at the
bottom. Which means, ``throw`` may not be used to to 'exit' out of the
structured block. If an exception is not caught in the same structured block
it was thrown in, the behaviour is undefined / implementation defined,
the program will likely terminate.
Reviewers: JonasToth, aaron.ballman, baloghadamsoftware, gribozavr
Reviewed By: aaron.ballman, gribozavr
Subscribers: mgorny, xazax.hun, rnkovacs, guansong, jdoerfert, cfe-commits, ABataev
Tags: #clang-tools-extra, #openmp, #clang
Differential Revision: https://reviews.llvm.org/D59466
llvm-svn: 356802
Summary:
Finds OpenMP directives that are allowed to contain `default` clause,
but either don't specify it, or the clause is specified but with the kind
other than `none`, and suggests to use `default(none)` clause.
Using `default(none)` clause changes the default variable visibility from
being implicitly determined, and thus forces developer to be explicit about the
desired data scoping for each variable.
Reviewers: JonasToth, aaron.ballman, xazax.hun, hokein, gribozavr
Reviewed By: JonasToth, aaron.ballman
Subscribers: jdoerfert, openmp-commits, klimek, sbenza, arphaman, Eugene.Zelenko, ABataev, mgorny, rnkovacs, guansong, cfe-commits
Tags: #clang-tools-extra, #openmp, #clang
Differential Revision: https://reviews.llvm.org/D57113
llvm-svn: 356801
Summary:
Add a way to expand modular headers for PPCallbacks. Checks can opt-in for this
expansion by overriding the new registerPPCallbacks virtual method and
registering their PPCallbacks in the preprocessor created for this specific
purpose.
Use module expansion in the readability-identifier-naming check
Reviewers: gribozavr, usaxena95, sammccall
Reviewed By: gribozavr
Subscribers: nemanjai, mgorny, xazax.hun, kbarton, jdoerfert, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D59528
llvm-svn: 356750
Summary:
In contrast to Google C++, Objective-C often uses built-in integer types
other than `int`. In fact, the Objective-C runtime itself defines the
types NSInteger¹ and NSUInteger² which are variant types depending on
the target architecture. The Objective-C style guide indicates that
usage of system types with variant sizes is appropriate when handling
values provided by system interfaces³. Objective-C++ is commonly the
result of conversion from Objective-C to Objective-C++ for the purpose
of integrating C++ functionality. The opposite of Objective-C++ being
used to expose Objective-C functionality to C++ is less common,
potentially because Objective-C has a signficantly more uneven presence
on different platforms compared to C++. This generally predisposes
Objective-C++ to commonly being more Objective-C than C++. Forcing
Objective-C++ developers to perform conversions between variant system types
and fixed size integer types depending on target architecture when
Objective-C++ commonly uses variant system types from Objective-C is
likely to lead to more bugs and overhead than benefit. For that reason,
this change proposes to disable google-runtime-int in Objective-C++.
[1] https://developer.apple.com/documentation/objectivec/nsinteger?language=objc
[2] https://developer.apple.com/documentation/objectivec/nsuinteger?language=objc
[3] "Types long, NSInteger, NSUInteger, and CGFloat vary in size between
32- and 64-bit builds. Use of these types is appropriate when handling
values exposed by system interfaces, but they should be avoided for most
other computations."
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#types-with-inconsistent-sizes
Subscribers: xazax.hun, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59336
llvm-svn: 356627
Summary:
-ignore specifies a list of PP callbacks to ignore. It cannot express a
whitelist, which may be more useful than a blacklist.
Add a new option -callbacks to replace it.
-ignore= (default) => -callbacks='*' (default)
-ignore=FileChanged,FileSkipped => -callbacks='*,-FileChanged,-FileSkipped'
-callbacks='Macro*' : print only MacroDefined,MacroExpands,MacroUndefined,...
Reviewers: juliehockett, aaron.ballman, alexfh, ioeric
Reviewed By: aaron.ballman
Subscribers: nemanjai, kbarton, jsji, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59296
llvm-svn: 356366
Makes the name of this directory consistent with the names of the other
directories in clang-tools-extra.
Differential Revision: https://reviews.llvm.org/D59382
llvm-svn: 356254
This is an analog of the abseil-duration-comparison check, but for the
absl::Time domain. It has a similar implementation and tests.
Differential Revision: https://reviews.llvm.org/D58977
llvm-svn: 355835
Summary:
The current install-clang-headers target installs clang's resource
directory headers. This is different from the install-llvm-headers
target, which installs LLVM's API headers. We want to introduce the
corresponding target to clang, and the natural name for that new target
would be install-clang-headers. Rename the existing target to
install-clang-resource-headers to free up the install-clang-headers name
for the new target, following the discussion on cfe-dev [1].
I didn't find any bots on zorg referencing install-clang-headers. I'll
send out another PSA to cfe-dev to accompany this rename.
[1] http://lists.llvm.org/pipermail/cfe-dev/2019-February/061365.html
Reviewers: beanz, phosek, tstellar, rnk, dim, serge-sans-paille
Subscribers: mgorny, javed.absar, jdoerfert, #sanitizers, openmp-commits, lldb-commits, cfe-commits, llvm-commits
Tags: #clang, #sanitizers, #lldb, #openmp, #llvm
Differential Revision: https://reviews.llvm.org/D58791
llvm-svn: 355340
Summary:
The usefulness of **modernize-use-override** can be reduced if you have to live in an environment where you support multiple compilers, some of which sadly are not yet fully C++11 compliant
some codebases have to use override as a macro OVERRIDE e.g.
```
// GCC 4.7 supports explicit virtual overrides when C++11 support is enabled.
```
This allows code to be compiled with C++11 compliant compilers and get warnings and errors that clang, MSVC,gcc can give, while still allowing other legacy pre C++11 compilers to compile the code. This can be an important step towards modernizing C++ code whilst living in a legacy codebase.
When it comes to clang tidy, the use of the **modernize-use-override** is one of the most useful checks, but the messages reported are inaccurate for that codebase if the standard approach is to use the macros OVERRIDE and/or FINAL.
When combined with fix-its that introduce the C++11 override keyword, they become fatal, resulting in the modernize-use-override check being turned off to prevent the introduction of such errors.
This revision, allows the possibility for the replacement **override **to be a macro instead, Allowing the clang-tidy check to be run on both pre and post C++11 code, and allowing fix-its to be applied.
Reviewers: alexfh, JonasToth, hokein, Eugene.Zelenko, aaron.ballman
Reviewed By: alexfh, JonasToth
Subscribers: lewmpk, malcolm.parsons, jdoerfert, xazax.hun, cfe-commits, llvm-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D57087
llvm-svn: 355132
Summary: Detect a few expressions as likely character expressions, see PR27723.
Reviewers: xazax.hun, alexfh
Subscribers: rnkovacs, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58609
llvm-svn: 355089
Summary:
Implicit functions are outside the control of source authors and should
be exempt from style restrictions.
Tested via running clang tools tests.
This is an amended followup to https://reviews.llvm.org/D57207
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: jdoerfert, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58095
llvm-svn: 354534
Summary: To take up the .clangd folder for other potential uses in the future.
Reviewers: kadircet, sammccall
Reviewed By: kadircet
Subscribers: ioeric, MaskRay, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58440
llvm-svn: 354505
"modernize-use-using" check is applicable only to C++11 and later. Spell it out
to avoid relying on default language version.
rdar://problem/47932196
llvm-svn: 354069
Summary:
Fixes https://bugs.llvm.org/show_bug.cgi?id=40593.
Non-percent-encoded chars doesn't cause any problems on the input-side since we
assume everything after authority section is data and don't interpret them as
delimeters.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58126
llvm-svn: 353857
Summary:
The modernize-use-default-member-init check crashes when trying to
create an assignment value for a value-initialized enum because it isn't a
BuiltinType.
An enum cannot be initialized by assigning 0 to it unless a cast is added.
It could be initialized with an enumerator with the value 0, but there might not
be one.
Avoid these issues by ignoring the UseAssignment setting for value-initialized
enums.
Fixes PR35050.
Reviewers: aaron.ballman, alexfh, JonasToth
Reviewed By: JonasToth
Subscribers: xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57852
llvm-svn: 353554
bugprone-argument-comment only supports identifying those comments which do not match the function parameter name
This revision add 3 options to adding missing argument comments to literals (granularity on type is added to control verbosity of fixit)
```
CheckOptions:
- key: bugprone-argument-comment.CommentBoolLiterals
value: '1'
- key: bugprone-argument-comment.CommentFloatLiterals
value: '1'
- key: bugprone-argument-comment.CommentIntegerLiterals
value: '1'
- key: bugprone-argument-comment.CommentStringLiterals
value: '1'
- key: bugprone-argument-comment.CommentCharacterLiterals
value: '1'
- key: bugprone-argument-comment.CommentUserDefinedLiterals
value: '1'
- key: bugprone-argument-comment.CommentNullPtrs
value: '1'
```
After applying these options, literal arguments will be preceded with /*ParameterName=*/
Reviewers: JonasToth, Eugene.Zelenko, alexfh, hokein, aaron.ballman
Reviewed By: aaron.ballman, Eugene.Zelenko
Differential Revision: https://reviews.llvm.org/D57674
llvm-svn: 353535
Summary:
The check should ignore the main function, the program entry point.
It is not possible to use `std::array<>` for the `argv`.
The alternative is to use `char** argv`.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=40604 | PR40604 ]]
Reviewers: JonasToth, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, hans, cfe-commits
Tags: #clang-tools-extra, #clang
Differential Revision: https://reviews.llvm.org/D57787
llvm-svn: 353327
Summary:
Implicit functions are outside the control of source authors and should
be exempt from style restrictions.
Tested via running clang tools tests.
Reviewers: aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D57207
llvm-svn: 352968
Summary:
This helps minimize the disruption of not returning declarations as part of
a find-definition response (r352864).
Reviewers: hokein
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, ilya-biryukov
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57580
llvm-svn: 352953
Summary:
LSP now reflects the declaration/definition distinction.
Language server changes:
- textDocument/definition now returns a definition if one is found, otherwise
the declaration. It no longer returns declaration + definition if they are
distinct.
- textDocument/declaration returns the best declaration we can find.
- For macros, the active macro definition is returned for both methods.
- For include directive, the top of the target file is returned for both.
There doesn't appear to be a discovery mechanism (we can't return everything to
clients that only know about definition), so this changes existing behavior.
In practice, it should greatly reduce the fraction of the time we need to show
the user a menu of options.
C++ API changes:
- findDefinitions is replaced by locateSymbolAt, which returns a
vector<LocatedSymbol> - one for each symbol under the cursor.
- this contains the preferred declaration, the definition (if found), and
the symbol name
This API enables some potentially-neat extensions, like swapping between decl
and def, and exposing the symbol name to the UI in the case of multiple symbols.
Reviewers: hokein
Subscribers: ilya-biryukov, javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D57388
llvm-svn: 352864
Summary:
This would make diagnostic fixits more discoverable, especially for
plugins like YCM.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D57509
llvm-svn: 352764
This should not happen in normal operation, as it implies that the diagnostics
with some available fixes were produced but the AST is invalid.
Moreover, the code had an error: always returned code actions ignoring the
SupportsCodeAction capability and writing a test for this is impossible,
since this can only happen due to programmer's error rather than invalid inputs.
llvm-svn: 352624
Summary:
The code tweaks are an implementation of mini-refactorings exposed
via the LSP code actions. They run in two stages:
- Stage 1. Decides whether the action is available to the user and
collects all the information required to finish the action.
Should be cheap, since this will run over all the actions known to
clangd on each textDocument/codeAction request from the client.
- Stage 2. Uses information from stage 1 to produce the actual edits
that the code action should perform. This stage can be expensive and
will only run if the user chooses to perform the specified action in
the UI.
One unfortunate consequence of this change is increased latency of
processing the textDocument/codeAction requests, which now wait for an
AST. However, we cannot avoid this with what we have available in the LSP
today.
Reviewers: kadircet, ioeric, hokein, sammccall
Reviewed By: sammccall
Subscribers: mgrang, mgorny, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D56267
llvm-svn: 352494
Summary:
Because _Float16 was disabled for X86 targets the unit-tests started failing.
Extract the pieces for _Float16 and run theses tests under AArch64.
Reviewers: aaron.ballman, erichkeane, lebedev.ri
Reviewed By: erichkeane
Subscribers: javed.absar, xazax.hun, kristof.beyls, cfe-commits
Differential Revision: https://reviews.llvm.org/D57249
llvm-svn: 352231
Seems the previous statement does not hold up anymore.
Reviewers: steveire
Reviewed By: steveire
Differential Revision: https://reviews.llvm.org/D57102
llvm-svn: 352031
The readability-else-after-return check should be smarter about cases where the
variable defined in the condition is used in the `else` branch. This patch makes
it just ignore such cases, but alternative solutions may be better (added a
FIXME).
llvm-svn: 351751
Otherwise we don't warn on a struct containing a single public int, but
we warn on a struct containing a single public std::string, which is
inconsistent.
llvm-svn: 351686
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
The underlying issue is fixed in https://reviews.llvm.org/D56444
and this test ensures the issue does not creep back into our
code-base.
Reviewers: alexfh, aaron.ballman, hokein, hwright
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D56918
llvm-svn: 351569
Summary:
Previously, we weren't recognizing these as smart pointers and thus
weren't allowing non-dereference accesses as we should -- see new test
cases which fail without the fix.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: JonasToth
Subscribers: xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D56585
llvm-svn: 351303
- New transport layer for macOS.
- XPC Framework
- Test client
Framework and client were written by Alex Lorenz.
Differential Revision: https://reviews.llvm.org/D54428
llvm-svn: 351280
Summary: Adds a checker to clang-tidy to warn when a non void const member function, taking only parameters passed by value or const reference could be marked as '[[nodiscard]]'
Patch by MyDeveloperDay.
Reviewers: alexfh, stephenkelly, curdeius, aaron.ballman, hokein, JonasToth
Reviewed By: curdeius, JonasToth
Subscribers: Eugene.Zelenko, lefticus, lebedev.ri, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55433
llvm-svn: 350760
Summary:
Only run completion when we were trigerred on '->' and '::', otherwise
send an error code in return.
To avoid automatically invoking completions in cases like 'a >^' or
'a ? b :^'.
Reviewers: hokein
Reviewed By: hokein
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D55994
llvm-svn: 350304
And also enable it by default to be consistent with e.g.
modernize-use-using.
This helps e.g. when running this check on client code where the macro
is provided by the system, so there is no easy way to modify it.
Reviewed By: JonasToth, lebedev.ri
Differential Revision: https://reviews.llvm.org/D56025
llvm-svn: 350056
Summary:
Previously, we'd only match on literal floating or integral zeroes, but I've now also learned that some users spell that value as int{0} or float{0}, which also need to be matched.
Differential Revision: https://reviews.llvm.org/D56012
llvm-svn: 349953
Summary:
This change relaxes the requirements on the utility
`rewriteExprFromNumberToDuration` function, and introduces new checking
inside of the `abseil-duration-comparison` check to allow macro argument
expression transformation.
Differential Revision: https://reviews.llvm.org/D55784
llvm-svn: 349636
Summary:
Currently, background index rebuilds symbol index on every indexed file,
which can be inefficient. This patch makes it only rebuild symbol index periodically.
As the rebuild no longer happens too often, we could also build more efficient
dex index.
Reviewers: ilya-biryukov, kadircet
Reviewed By: kadircet
Subscribers: dblaikie, MaskRay, jkorous, arphaman, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D55770
llvm-svn: 349496
Summary:
The diagnostics from google-objc-function-naming check will be more
actionable if they provide a brief description of the requirements from
the Google Objective-C style guide. The more descriptive diagnostics may
help clarify that functions in the global namespace must have an
appropriate prefix followed by Pascal case (engineers working previously
with static functions might not immediately understand the different
requirements of static and non-static functions).
Test Notes:
Verified against the clang-tidy tests.
Reviewers: benhamilton, aaron.ballman
Reviewed By: benhamilton
Subscribers: MyDeveloperDay, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D55482
llvm-svn: 349123
Summary:
This check uses the context of a subtraction expression as well as knowledge
about the Abseil Time types, to infer the type of the second operand of some
subtraction expressions in Duration conversions. For example:
absl::ToDoubleSeconds(duration) - foo
can become
absl::ToDoubleSeconds(duration - absl::Seconds(foo))
This ensures that time calculations are done in the proper domain, and also
makes it easier to further deduce the types of the second operands to these
expressions.
Reviewed By: JonasToth
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55245
llvm-svn: 349073
Summary: Several tests re-implement these same prototypes (differently), so we can put them in a common location.
Patch by hwright.
Reviewers: JonasToth
Reviewed By: JonasToth
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55540
llvm-svn: 348840
Patch by Alex Strelnikov.
Reviewed as D53830
Introduce a new check to upgrade user code based on upcoming API breaking changes to absl::Duration.
The check finds calls to arithmetic operators and factory functions for absl::Duration that rely on
an implicit user defined conversion to int64_t. These cases will no longer compile after proposed
changes are released. Suggested fixes explicitly cast the argument int64_t.
llvm-svn: 348633
Summary:
The intention is to make the tools replaying compilations from 'compile_commands.json'
(clang-tidy, clangd, etc.) find the same standard library as the original compiler
specified in 'compile_commands.json'.
Previously, the library detection logic was in the frontend (InitHeaderSearch.cpp) and relied
on the value of resource dir as an approximation of the compiler install dir. The new logic
uses the actual compiler install dir and is performed in the driver. This is consistent with
the C++ standard library detection on other platforms and allows to override the resource dir
in the tools using the compile_commands.json without altering the
standard library detection mechanism. The tools have to override the resource dir to make sure
they use a consistent version of the builtin headers.
There is still logic in InitHeaderSearch that attemps to add the absolute includes for the
the C++ standard library, so we keep passing the -stdlib=libc++ from the driver to the frontend
via cc1 args to avoid breaking that. In the long run, we should move this logic to the driver too,
but it could potentially break the library detection on other systems, so we don't tackle it in this
patch to keep its scope manageable.
This is a second attempt to fix the issue, first one was commited in r346652 and reverted in r346675.
The original fix relied on an ad-hoc propagation (bypassing the cc1 flags) of the install dir from the
driver to the frontend's HeaderSearchOptions. Unsurpisingly, the propagation was incomplete, it broke
the libc++ detection in clang itself, which caused LLDB tests to break.
The LLDB tests pass with new fix.
Reviewers: JDevlieghere, arphaman, EricWF
Reviewed By: arphaman
Subscribers: mclow.lists, ldionne, dexonsmith, ioeric, christof, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54630
llvm-svn: 348365
Summary:
Implement a check for detecting if/else if/else chains where two or more
branches are Type I clones of each other (that is, they contain identical code)
and for detecting switch statements where two or more consecutive branches are
Type I clones of each other.
Patch by donat.nagy.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: JonasToth
Subscribers: MTC, lebedev.ri, whisperity, xazax.hun, Eugene.Zelenko, mgorny, rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D54757
llvm-svn: 348343
Summary:
bugprone-misplaced-widening-cast check
used to give a false warning to the
following example.
enum DaysEnum{
MON = 0,
TUE = 1
};
day = (DaysEnum)(day + 1);
//warning: either cast from 'int' to 'DaysEnum' is ineffective...
But i think int to enum cast is not widening neither ineffective.
Patch by dkrupp.
Reviewers: JonasToth, alexfh
Reviewed By: alexfh
Subscribers: rnkovacs, Szelethus, gamesh411, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55255
llvm-svn: 348341
Summary:
§1 Description
This changes the objc-property-declaration check to allow arbitrary acronyms and initialisms instead of using whitelisted acronyms. In Objective-C it is relatively common to use project prefixes in property names for the purposes of disambiguation. For example, the CIColor¹ and CGColor² properties on UIColor both represent symbol prefixes being used in proeprty names outside of Apple's accepted acronyms³. The union of Apple's accepted acronyms and all symbol prefixes that might be used for disambiguation in property declarations effectively allows for any arbitrary sequence of capital alphanumeric characters to be acceptable in property declarations. This change updates the check accordingly.
The test variants with custom configurations are deleted as part of this change because their configurations no longer impact behavior. The acronym configurations are currently preserved for backwards compatibility of check configuration.
[1] https://developer.apple.com/documentation/uikit/uicolor/1621951-cicolor?language=objc
[2] https://developer.apple.com/documentation/uikit/uicolor/1621954-cgcolor?language=objc
[3] https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
§2 Test Notes
Changes verified by:
• Running clang-tidy unit tests.
• Used check_clang_tidy.py to verify expected output of processing objc-property-declaration.m
Reviewers: benhamilton, Wizard
Reviewed By: benhamilton
Subscribers: jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D51832
llvm-svn: 348331
Summary: The google-objc-function-naming check applies to functions that are not namespaced and should not be applied to C++ member functions. Such function declarations should be ignored by the check to avoid false positives in Objective-C++ sources.
Reviewers: benhamilton, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D55101
llvm-svn: 348317
Summary:
This check finds instances where Duration values are being converted to a numeric value in a comparison expression, and suggests that the conversion happen on the other side of the expression to a Duration. See documentation for examples.
This also shuffles some code around so that the new check may perform in sone step simplifications also caught by other checks.
Compilation is unbroken, because the hash-function is now directly
specified for std::unordered_map, as 'enum class' does not compile as
key (seamingly only on some compilers).
Patch by hwright.
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: JonasToth
Subscribers: sammccall, Eugene.Zelenko, xazax.hun, cfe-commits, mgorny
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D54737
llvm-svn: 348169
Summary:
This check finds instances where Duration values are being converted to a numeric value in a comparison expression, and suggests that the conversion happen on the other side of the expression to a Duration. See documentation for examples.
This also shuffles some code around so that the new check may perform in sone step simplifications also caught by other checks.
Patch by hwright.
Reviewers: aaron.ballman, JonasToth, alexfh, hokein
Reviewed By: JonasToth
Subscribers: sammccall, Eugene.Zelenko, xazax.hun, cfe-commits, mgorny
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D54737
llvm-svn: 348161
This check is about preventing exceptions from being thrown before main() executes, and assigning a lambda (rather than calling it) to a global object cannot throw any exceptions.
llvm-svn: 347761
Summary:
Include IndexBenchmark in check-clangd to make sure we won't forget to update
it when doing breaking changes; also fix an out-of-date test input.
Reviewers: ilya-biryukov
Subscribers: mgorny, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54998
llvm-svn: 347753
If a variable is declared constexpr then its initializer needs to be a constant expression, and thus, cannot throw. This check is about not throwing exceptions before main() runs, and so it doesn't apply if the initializer cannot throw. This silences the diagnostic when initializing a constexpr variable and fixes PR35457.
llvm-svn: 347745
New method returning symbol info for given source position.
Differential Revision: https://reviews.llvm.org/D54799
rdar://problem/46050281
llvm-svn: 347675
Summary: There is no ambiguity / information loss in this conversion
Reviewers: alexfh, aaron.ballman, hokein
Reviewed By: alexfh
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54941
llvm-svn: 347671
When a warning is issued in a template instantiation, the check would previously
use template arguments in a note, which would result in inconsistent or
duplicate warnings (depending on how deduplication was done). This patch removes
template arguments from the note.
llvm-svn: 347652
Summary:
Ownership and configuration:
The auto-index (background index) is maintained by ClangdServer, like Dynamic.
(This means ClangdServer will be able to enqueue preamble indexing in future).
For now it's enabled by a simple boolean flag in ClangdServer::Options, but
we probably want to eventually allow injecting the storage strategy.
New 'sync' command:
In order to meaningfully test the integration (not just unit-test components)
we need a way for tests to ensure the asynchronous index reads/writes occur
before a certain point.
Because these tests and assertions are few, I think exposing an explicit "sync"
command for use in tests is simpler than allowing threading to be completely
disabled in the background index (as we do for TUScheduler).
Bugs:
I fixed a couple of trivial bugs I found while testing, but there's one I can't.
JSONCompilationDatabase::getAllFiles() may return relative paths, and currently
we trigger an assertion that assumes they are absolute.
There's no efficient way to resolve them (you have to retrieve the corresponding
command and then resolve against its directory property). In general I think
this behavior is broken and we should fix it in JSONCompilationDatabase and
require CompilationDatabase::getAllFiles() to be absolute.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D54894
llvm-svn: 347567
Summary: The fix for `auto` new expression is illegal.
Reviewers: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54832
llvm-svn: 347551
Summary:
The fix for aggregate initialization (`std::make_unique<Foo>(Foo {1, 2})` needs
to see Foo copy constructor, otherwise we will have a compiler error. So we
only emit the check warning.
Reviewers: JonasToth, aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54745
llvm-svn: 347537
The test I'm adding passes without the change due to the deduplication logic in
ClangTidyDiagnosticConsumer::take(). However this bug manifests in our internal
integration with clang-tidy.
I've verified the fix by locally changing LessClangTidyError to consider
replacements.
llvm-svn: 347470
Summary:
Instead of passing around a list of supported URI schemes in clangd, we
expose an interface to convert a path to URI using any compatible scheme
that has been registered. It favors customized schemes and falls
back to "file" when no other scheme works.
Changes in this patch are:
- URI::create(AbsPath, URISchemes) -> URI::create(AbsPath). The new API finds a
compatible scheme from the registry.
- Remove URISchemes option everywhere (ClangdServer, SymbolCollecter, FileIndex etc).
- Unit tests will use "unittest" by default.
- Move "test" scheme from ClangdLSPServer to ClangdMain.cpp, and only
register the test scheme when lit-test or enable-lit-scheme is set.
(The new flag is added to make lit protocol.test work; I wonder if there
is alternative here.)
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54800
llvm-svn: 347467
Summary:
The full path of the input header depends on the execution environment
and may result in different behavior (e.g. when different URI schemes are used).
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54833
llvm-svn: 347466
Summary:
Currently the smart_ptr check (modernize-make-unique) generates the
fixes that cannot compile for cases like below -- because brace list can
not be deduced in `make_unique`.
```
struct Bar { int a, b; };
struct Foo { Foo(Bar); };
auto foo = std::unique_ptr<Foo>(new Foo({1, 2}));
```
Reviewers: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54704
llvm-svn: 347315
Summary:
§1 Description
This check finds function names in function declarations in Objective-C files that do not follow the naming pattern described in the Google Objective-C Style Guide. Function names should be in UpperCamelCase and functions that are not of static storage class should have an appropriate prefix as described in the Google Objective-C Style Guide. The function `main` is a notable exception. Function declarations in expansions in system headers are ignored.
Example conforming function definitions:
```
static bool IsPositive(int i) { return i > 0; }
static bool ABIsPositive(int i) { return i > 0; }
bool ABIsNegative(int i) { return i < 0; }
```
A fixit hint is generated for functions of static storage class but otherwise the check does not generate a fixit hint because an appropriate prefix for the function cannot be determined.
§2 Test Notes
* Verified clang-tidy tests pass successfully.
* Used check_clang_tidy.py to verify expected output of processing google-objc-function-naming.m
Reviewers: benhamilton, hokein, Wizard, aaron.ballman
Reviewed By: benhamilton
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51575
llvm-svn: 347132
Summary:
[[ https://bugs.llvm.org/show_bug.cgi?id=39224 | PR39224 ]]
As discussed, we can't always do the transform automatically due to that array-to-pointer decay of C array.
In order to detect whether we can do said transform, we'd need to be able to see all usages of said array,
which is, i would say, rather impossible if e.g. it is in the header.
Thus right now no fixit exists.
Exceptions: `extern "C"` code.
References:
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es27-use-stdarray-or-stack_array-for-arrays-on-the-stack | CPPCG ES.27: Use std::array or stack_array for arrays on the stack ]]
* [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon1-prefer-using-stl-array-or-vector-instead-of-a-c-array | CPPCG SL.con.1: Prefer using STL array or vector instead of a C array ]]
* HICPP `4.1.1 Ensure that a function argument does not undergo an array-to-pointer conversion`
* MISRA `5-2-12 An identifier with array type passed as a function argument shall not decay to a pointer`
Reviewers: aaron.ballman, JonasToth, alexfh, hokein, xazax.hun
Reviewed By: JonasToth
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53771
llvm-svn: 346835
The new checker searches for those for loops which has a loop variable with a "too small" type which means this type can't represent all values which are part of the iteration range.
For example:
```
int main() {
long size = 300000;
for( short int i = 0; i < size; ++i) {}
}
```
The short type leads to infinite loop here because it can't store all values in the `[0..size]` interval. In a real use case, size means a container's size which depends on the user input. Which means for small amount of objects the algorithm works, but with a larger user input the software will freeze.
The idea of the checker comes from the LibreOffice project, where the same check was implemented as a clang compiler plugin, called `LoopVarTooSmall` (LLVM licensed).
The idea is the same behind this check, but the code is different because of the different framework.
Patch by ztamas.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth, xazax.hun, whisperity
Reviewed By: JonasToth, whisperity
Differential Revision: https://reviews.llvm.org/D53974
llvm-svn: 346665
Summary:
The fix to the issue that `const char* p = ("foo")` is diagnosed as decay
is to ignored the ParenCast.
Resolves PR39583
Reviewers: aaron.ballman, alexfh, hokein
Reviewed By: aaron.ballman
Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits
Differential Revision: https://reviews.llvm.org/D54281
llvm-svn: 346555
r345961 introduced a path check in
.\tools\clang\tools\extra\test\clang-tidy\clang-tidy-run-with-database.cpp.
r345961 added a check line for a path that only handled / on unix
machines and not \ on windows machines.
This patch handles both cases.
Patch by TWeaver.
Differential Revision: https://reviews.llvm.org/D54036
llvm-svn: 345995
Summary:
This was added in D34947 to support YCM, but YCM actually provides *all* args,
and this was never actually used.
Meanwhile, we grew another extension that allows specifying all args.
I did find one user of this extension: https://github.com/thomasjo/atom-ide-cpp.
I'll reach out, there are multiple good alternatives:
- compile_commands.txt can serve the same purpose as .clang_complete there
- we can add an extension to support setting the fallback command
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53641
llvm-svn: 345969
Summary:
Currently ClangTidyContext::diag() sends the diagnostics to a
DiagnosticsEngine, which probably delegates to a ClangTidyDiagnosticsConsumer,
which is supposed to go back and populate ClangTidyContext::Errors.
After this patch, the diagnostics are stored in the ClangTidyDiagnosticsConsumer
itself and can be retrieved from there.
Why?
- the round-trip from context -> engine -> consumer -> context is confusing
and makes it harder to establish layering between these things.
- context does too many things, and makes it hard to use clang-tidy as a library
- everyone who actually wants the diagnostics has access to the ClangTidyDiagnosticsConsumer
The most natural implementation (ClangTidyDiagnosticsConsumer::take()
finalizes diagnostics) causes a test failure: clang-tidy-run-with-database.cpp
asserts that clang-tidy exits successfully when trying to process a file
that doesn't exist.
In clang-tidy today, this happens because finish() is never called, so the
diagnostic is never flushed. This looks like a bug to me.
For now, this patch carefully preserves that behavior, but I'll ping the
authors to see whether it's deliberate and worth preserving.
Reviewers: hokein
Subscribers: xazax.hun, cfe-commits, alexfh
Differential Revision: https://reviews.llvm.org/D53953
llvm-svn: 345961
Summary: This will make clang-tidy accept property names like xyz_URL (URL is a common acronym).
Reviewers: benhamilton, hokein
Reviewed By: benhamilton
Subscribers: jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D53955
llvm-svn: 345858
This check flags function top-level const-qualified return types and suggests removing the mostly-superfluous const qualifier where possible.
Patch by Yitzhak Mandelbaum.
llvm-svn: 345764
Summary:
This patch introduces a new clang-tidy check that matches on all `declStmt` that declare more then one variable
and transform them into one statement per declaration if possible.
It currently only focusses on variable declarations but should be extended to cover more kinds of declarations in the future.
It is related to https://reviews.llvm.org/D27621 and does use it's extensive test-suite. Thank you to firolino for his work!
Reviewers: rsmith, aaron.ballman, alexfh, hokein, kbobyrev
Reviewed By: aaron.ballman
Subscribers: ZaMaZaN4iK, mgehre, nemanjai, kbarton, lebedev.ri, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D51949
llvm-svn: 345735
Summary:
The macro may not have location (or more generally, the location may not exist),
e.g. if it originates from compiler's command-line.
The check complains on all the macros, even those without the location info.
Which means, it only says it does not like it. What is 'it'? I have no idea.
If we don't print the name, then there is no way to deal with that situation.
And in general, not printing name here forces the user to try to understand,
given, the macro definition location, what is the macro name?
This isn't fun.
Also, ignores-by-default the macros originating from command-line,
with an option to not ignore those.
I suspect some more issues may crop up later.
Reviewers: JonasToth, aaron.ballman, hokein, xazax.hun, alexfh
Reviewed By: JonasToth, aaron.ballman
Subscribers: nemanjai, kbarton, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53817
llvm-svn: 345610
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
This is a re-commit, the original was reverted by me in
rL345305 due to discovered bugs. (implicit code, template instantiation)
Tests were added, and the bugs were fixed.
I'm unable to find any further bugs, hopefully there aren't any..
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
llvm-svn: 345381
There are some lurking issues with the handling of the SourceManager.
Somehow sometimes we end up extracting completely wrong
portions of the source buffer.
Reverts r344772, r44760, r344758, r344755.
llvm-svn: 345305
Summary:
This check finds cases where calls to an absl::Duration factory could use the more efficient integer overload.
For example:
// Original - Providing a floating-point literal.
absl::Duration d = absl::Seconds(10.0);
// Suggested - Use an integer instead.
absl::Duration d = absl::Seconds(10);
Patch by hwright.
Reviewers: alexfh, hokein, aaron.ballman, JonasToth
Reviewed By: hokein, JonasToth
Subscribers: zturner, xazax.hun, Eugene.Zelenko, mgorny, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53339
llvm-svn: 345167
Summary:
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.
After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.
Reviewers: hokein, arphaman
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53391
llvm-svn: 345119
And also enable it by default to be consistent with e.g. modernize-use-using.
This helps e.g. when running this check on client code where the macro is
provided by the system, so there is no easy way to modify it.
Reviewed By: JonasToth
Differential Revision: https://reviews.llvm.org/D53454
llvm-svn: 344871
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
llvm-svn: 344755
Summary:
LSP is a slightly awkward map to C++ object lifetimes: the initialize request
is part of the protocol and provides information that doesn't change over the
lifetime of the server.
Until now, we handled this by initializing ClangdServer and ClangdLSPServer
right away, and making anything that can be set in the "initialize" request
mutable.
With this patch, we create ClangdLSPServer immediately, but defer creating
ClangdServer until "initialize". This opens the door to passing the relevant
initialize params in the constructor and storing them immutably.
(That change isn't actually done in this patch).
To make this safe, we have the MessageDispatcher enforce that the "initialize"
method is called before any other (as required by LSP). That way each method
handler can assume Server is initialized, as today.
As usual, while implementing this I found places where our test cases violated
the protocol.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53398
llvm-svn: 344741
Summary:
Most of its functionality is moved into ClangdLSPServer.
The decoupling between JSONRPCDispatcher, ProtocolCallbacks, ClangdLSPServer
was never real, and only served to obfuscate.
Some previous implicit/magic stuff is now explicit:
- the return type of LSP method calls are now in the signature
- no more reply() that gets the ID using global context magic
- arg tracing no longer relies on RequestArgs::stash context magic either
This is mostly refactoring, but some deliberate fixes while here:
- LSP method params are now by const reference
- notifications and calls are now distinct namespaces.
(some tests had protocol errors and needed updating)
- we now reply to calls we failed to decode
- outgoing calls use distinct IDs
A few error codes and message IDs changed in unimportant ways (see tests).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53387
llvm-svn: 344737
Summary:
Previously, ptr.reset(new char[5]) will be replaced with `p =
make_unique<char[]>(5)`, the fix has side effect -- doing
default initialization, it may cause performace regression (we are
bitten by this rececntly)
The check should be conservative for these cases.
Reviewers: alexfh
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D53377
llvm-svn: 344733
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.
This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.
The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.
Reviewers: jkorous, ioeric, hokein
Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53286
llvm-svn: 344672
Nico Weber