JonasToth (Jonas Toth)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 31 2016, 11:13 AM (63 w, 2 d)

Recent Activity

Today

JonasToth committed rCTE322626: [clang-tidy] implement check for goto.
[clang-tidy] implement check for goto
Wed, Jan 17, 2:29 AM
JonasToth committed rL322626: [clang-tidy] implement check for goto.
[clang-tidy] implement check for goto
Wed, Jan 17, 2:29 AM
JonasToth closed D41815: [clang-tidy] implement check for goto.
Wed, Jan 17, 2:29 AM
JonasToth committed rCTE322624: [clang-tidy] fix minor formatting issue.
[clang-tidy] fix minor formatting issue
Wed, Jan 17, 2:25 AM
JonasToth committed rL322624: [clang-tidy] fix minor formatting issue.
[clang-tidy] fix minor formatting issue
Wed, Jan 17, 2:25 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • [Misc] remove spurious format
Wed, Jan 17, 2:22 AM

Mon, Jan 15

JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • last nits i found
Mon, Jan 15, 12:56 PM
JonasToth added a comment to D41648: [clang-tidy] implement cppcoreguidelines macro rules.

My gut reaction is still that this check is going to be too chatty on real world code bases to be of value beyond compliance with the C++ Core Guidelines, but that's also sufficient enough reason to have the check in the first place. Perhaps a reasonable approach would be to put in the heuristics you think make the most sense, then run the check over some large real world C++ code bases to see how many diagnostics are generated. That will also help you to see code patterns to modify your heuristic until you're happy with the results. Then we can debate whether there's more left to do from the data.

Mon, Jan 15, 12:50 PM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • remove spurious whitespace
Mon, Jan 15, 12:48 PM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • hicpp alias added
  • update documentation
Mon, Jan 15, 12:46 PM
JonasToth added a comment to D41648: [clang-tidy] implement cppcoreguidelines macro rules.

Using regex could be a reasonable way out, but isn't going to solve the problem in general because not all of the macro names are ones the user has control over. Having to whitelist dozens of macros by name means this check is simply going to be disabled by the user as being low-value.

Yes. I think with the regex its exactly implemented like the rules say. But adding sugar is still reasonable. I will run the check over llvm to get a feeling how much is still left after silence LLVM_* and others that are clearly scoped.

Mon, Jan 15, 11:52 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • address eugene comments
Mon, Jan 15, 11:16 AM
JonasToth added inline comments to D41648: [clang-tidy] implement cppcoreguidelines macro rules.
Mon, Jan 15, 11:16 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • doc fix
Mon, Jan 15, 11:09 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • address review comments
Mon, Jan 15, 11:08 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • [Feature] implement regex for allowed macros
  • [Test] add proper tests for regex silencing
Mon, Jan 15, 10:54 AM
JonasToth added inline comments to D41655: [clang-tidy] New check bugprone-unused-return-value.
Mon, Jan 15, 9:07 AM · Restricted Project
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • doc clarified that check is c++ only
  • add missing tests for do and range-for, not all combinations done, but every loop construct is used as outer and inner loop
Mon, Jan 15, 8:45 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • [Fix] bug with language mode
Mon, Jan 15, 8:30 AM
JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Mon, Jan 15, 8:27 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • address review comments
Mon, Jan 15, 8:27 AM

Sat, Jan 13

JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Sat, Jan 13, 8:23 AM
JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Sat, Jan 13, 6:40 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • add edgecase test from roman
Sat, Jan 13, 6:40 AM

Fri, Jan 12

JonasToth added inline comments to D41655: [clang-tidy] New check bugprone-unused-return-value.
Fri, Jan 12, 1:13 PM · Restricted Project
JonasToth updated the diff for D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic.

rebase to 7.0.0

Fri, Jan 12, 12:57 PM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • minor issues fixed
Fri, Jan 12, 12:53 PM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • address review comments
Fri, Jan 12, 12:49 PM
JonasToth updated the diff for D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.
  • get up to date to 7.0
Fri, Jan 12, 12:35 PM
JonasToth added a comment to D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.

@sbenza and/or @klimek did you have time to address the stackoverflow caused from the ASTMatchers?

Fri, Jan 12, 12:30 PM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • simplified the code and merged diagnostics
Fri, Jan 12, 12:28 PM
JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Fri, Jan 12, 12:24 PM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • address review comments
  • add better documentation with code examples
Fri, Jan 12, 11:58 AM
JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Fri, Jan 12, 11:10 AM
JonasToth added a comment to D39865: Use default member initialization where possible.

Manually. Is there clang-tidy method of doing this? This change is pretty conservative, it only includes to more obvious cases I could find.

Fri, Jan 12, 9:20 AM
JonasToth added a comment to D41655: [clang-tidy] New check bugprone-unused-return-value.

High Integrity C++ has the rule 17.5.1 Do not ignore the result of std::remove, std::remove_if or std::unique. Do we want to add those to the preconfigured list?

Fri, Jan 12, 9:17 AM · Restricted Project

Thu, Jan 11

JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Thu, Jan 11, 1:57 PM
JonasToth added a comment to D41815: [clang-tidy] implement check for goto.
  • check that jumps will only be forward. AFAIK that is required in all sensefull usecases of goto, is it?

You could implement loops/recursion with goto, something like:

const int end = 10;
int i = 0;
assert(i < end);

begin:
<do stuff>
i++
if(i < end)
  goto begin;

// end

But it really should be done with normal for(), or while(), so i think it would make sense to diagnose those.

Thu, Jan 11, 1:50 PM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.

I enhanced the check to do more:

Thu, Jan 11, 4:53 AM
JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Thu, Jan 11, 4:24 AM

Mon, Jan 8

JonasToth added inline comments to D41815: [clang-tidy] implement check for goto.
Mon, Jan 8, 5:58 AM
JonasToth added a comment to D41815: [clang-tidy] implement check for goto.

Rather than add a warning for the labels, I would just add a note for the label when diagnosing the goto (since the goto has a single target).

Mon, Jan 8, 5:47 AM
JonasToth added a comment to D41815: [clang-tidy] implement check for goto.

A high level comment: while it is very easy to get all the gotos using grep, it is not so easy to get all the labels. So for this check to be complete, I think it would be useful to also find labels (possibly having a configuration option for that).

Mon, Jan 8, 5:21 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • [Misc] reformat
Mon, Jan 8, 2:47 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • [Misc] emphasize goto in warning message
Mon, Jan 8, 1:55 AM
JonasToth updated the diff for D41815: [clang-tidy] implement check for goto.
  • fix typos and return type of main in test
Mon, Jan 8, 1:54 AM
JonasToth created D41815: [clang-tidy] implement check for goto.
Mon, Jan 8, 1:38 AM

Sun, Jan 7

JonasToth added a comment to D41648: [clang-tidy] implement cppcoreguidelines macro rules.

Yes, and I'm saying that the guidelines aren't useful for real code bases because they restrict more than is reasonable. So while I think the check is largely implementing what the guidelines recommend, I think that some of these scenarios should be brought back to the guideline authors to weigh in on before we expose the check to users.

I see. Are u willing to identify some of the exceptions with me or don't you have enough time? I would like to propose the first list here. My opinion is, that we should leave some useful macros (e.g. Ensure from guidelines) left for manual inspection and to disable the check for these specific macros.

Sun, Jan 7, 7:32 AM
JonasToth retitled D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test from [clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test to [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.
Sun, Jan 7, 4:31 AM
JonasToth updated the diff for D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic.
  • rebase after release of 6.0
Sun, Jan 7, 4:31 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • rebase after release for 6.0
Sun, Jan 7, 4:29 AM

Sat, Jan 6

JonasToth added inline comments to D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t.
Sat, Jan 6, 3:08 AM · Restricted Project
JonasToth added a comment to D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t.

Could you please add a test case with a template that reduces the type to int8 or uint8?

I don't actually know how to do that. I tried a few things, but getting the type of the expression through a template gets me directly to unsigned char, not to uint8_t.

Sat, Jan 6, 3:03 AM · Restricted Project

Thu, Jan 4

JonasToth added inline comments to D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t.
Thu, Jan 4, 11:28 PM · Restricted Project
JonasToth added a comment to D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t.

Could you please add a test case with a template that reduces the type to int8 or uint8?

Thu, Jan 4, 11:15 PM · Restricted Project

Tue, Jan 2

JonasToth added inline comments to D41665: [Docs] Add Contributing page..
Tue, Jan 2, 6:32 AM

Mon, Jan 1

JonasToth added a comment to D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic.

Guidelines issue is here: https://github.com/isocpp/CppCoreGuidelines/issues/1120

Mon, Jan 1, 7:32 AM
JonasToth added a comment to D41655: [clang-tidy] New check bugprone-unused-return-value.

I think it would be more user friendly if the configured list can be a list and the | concatenation is done within your code.

Mon, Jan 1, 5:12 AM · Restricted Project

Sun, Dec 31

JonasToth added a comment to D41648: [clang-tidy] implement cppcoreguidelines macro rules.

I think this check is going to be extraordinarily chatty. For instance, macros are often used to hide compiler details in signatures, such as use of attributes. This construct cannot be replaced with anything else because the macro isn't defining an object or value. Another example where this will be bad is for conditional processing where the macro is later used in a #if, #elif, #ifdef, or #ifndef preprocessor conditional, as this also cannot be replaced with a constexpr variable. Without handling things like this, I don't see how this rule can provide utility to real world code. Do you have ideas for how to handle these kind of situations?

Sun, Dec 31, 9:14 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • remove unneeded includes
Sun, Dec 31, 7:37 AM
JonasToth updated the diff for D41648: [clang-tidy] implement cppcoreguidelines macro rules.
  • merge with local repos
Sun, Dec 31, 7:34 AM
JonasToth updated the summary of D41648: [clang-tidy] implement cppcoreguidelines macro rules.
Sun, Dec 31, 7:31 AM
JonasToth created D41648: [clang-tidy] implement cppcoreguidelines macro rules.
Sun, Dec 31, 7:29 AM

Fri, Dec 29

JonasToth added a comment to D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.

@sbenza and/or @klimek did you have time to address the stackoverflow caused from the ASTMatchers?

Fri, Dec 29, 10:46 AM

Sat, Dec 23

JonasToth added a comment to D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects.

What happens for c++98? I realize that fuchsia is c++14 but we might still think about not having constexpr.
If we just assume c++11 you can do the matching only for it. (getLangOpts or similar, see other checks for it.)

Sat, Dec 23, 2:36 AM · Restricted Project
JonasToth added a comment to D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects.

Is the singleton allowed, that uses a static object in the instance method?

Sat, Dec 23, 2:31 AM · Restricted Project

Thu, Dec 21

JonasToth added inline comments to D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators.
Thu, Dec 21, 12:34 AM · Restricted Project

Dec 18 2017

JonasToth added a comment to D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators.

What happens if the operator is overloaded outside a class? Is that allowed/disallowed and could you please mention the guideline on that in the docs + tests.

Dec 18 2017, 11:36 AM · Restricted Project

Dec 16 2017

JonasToth added a comment to D41308: [analyser] different.BitwiseOpBoolArg checker implementation.

That check is similar to 'hicpp-signed-bitwise' in clang-tidy. Maybe it could live there and extend it?

Dec 16 2017, 3:08 AM

Dec 13 2017

JonasToth added a comment to D41102: Setup clang-doc frontend framework.

I am happy now. But I don't have any authority to allow this patch to land whatsoever. Who will be the code owner for clang-doc? I think the tooling guys need to accept.

Dec 13 2017, 10:18 PM · Restricted Project
JonasToth added inline comments to D41102: Setup clang-doc frontend framework.
Dec 13 2017, 11:53 AM · Restricted Project
JonasToth added a comment to D41102: Setup clang-doc frontend framework.

You can erase some namespace prefix in code, e.g. llvm or clang, because you are working in them and/or had using namespace clang. Imo this is not required only if you find it ok to shorten the code.

Dec 13 2017, 12:39 AM · Restricted Project

Dec 12 2017

JonasToth added a comment to D41102: Setup clang-doc frontend framework.

Hi julie,

Dec 12 2017, 3:04 AM · Restricted Project
JonasToth added a comment to D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval.

What do you think about making this check fully configurable through the check options? The options could list the functions to check. The documentation could suggest some candidate functions from std lib to configure checks for.

Dec 12 2017, 2:18 AM · Restricted Project

Dec 10 2017

JonasToth added a comment to D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic.

The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so it might be worth bringing it up as an issue on their bug tracker. ES.100 basically says "you know what we mean, wink wink" as enforcement and doesn't give any guidance as to what is safe or unsafe. It gives no exceptions, which I think is unlikely to be useful to most developers. For instance: void f(unsigned i) { if (i > 0) {} }, this is a mixed signed and unsigned comparison (literals are signed by default) -- should this check diagnose?

Dec 10 2017, 12:02 PM
JonasToth added a comment to D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval.

May be bugprone is better module then misc?

Maybe. I can move it if all the reviewers think that it would be better suited there.

Dec 10 2017, 11:09 AM · Restricted Project

Dec 5 2017

JonasToth updated the diff for D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic.
  • [Misc] remove iostream
Dec 5 2017, 2:17 PM
JonasToth created D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic.
Dec 5 2017, 2:15 PM
JonasToth added a comment to D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.

@sbenza and @klimek the issue that occurs here is probably ASTMatchers related, thats why I added you.

Dec 5 2017, 9:46 AM
JonasToth added a reviewer for D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test: klimek.
Dec 5 2017, 9:39 AM
JonasToth updated the diff for D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.
  • [Misc] comment out matcher, make compile and runnable
Dec 5 2017, 9:33 AM
JonasToth added a reviewer for D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test: sbenza.
Dec 5 2017, 9:31 AM
JonasToth committed rCTE319785: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation.
[clang-tidy] adjust cppcoreguidelines-owning-memory documentation
Dec 5 2017, 8:38 AM
JonasToth committed rL319785: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation.
[clang-tidy] adjust cppcoreguidelines-owning-memory documentation
Dec 5 2017, 8:38 AM
JonasToth closed D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation by committing rL319785: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation.
Dec 5 2017, 8:38 AM
JonasToth updated the diff for D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation.
  • [Fix] typo
Dec 5 2017, 8:35 AM
JonasToth added a comment to D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation.

I agree.

Dec 5 2017, 8:35 AM
JonasToth added a comment to D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.

@aaron.ballman I further investigated and the issue does not come directly from my code (at least thats what i believe) :)

Dec 5 2017, 3:47 AM
JonasToth created D40829: [clang-tidy] adjust cppcoreguidelines-owning-memory documentation.
Dec 5 2017, 2:40 AM

Dec 4 2017

JonasToth added inline comments to D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Dec 4 2017, 11:23 AM · Restricted Project
JonasToth added inline comments to D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Dec 4 2017, 8:41 AM · Restricted Project
JonasToth retitled D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions from Replace the usage of std::uncaught_exception with std::uncaught_exceptions to [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Dec 4 2017, 8:31 AM · Restricted Project
JonasToth retitled D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test from [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test to [clang-tidy] WIP Resubmit hicpp-multiway-paths-covered without breaking test .
Dec 4 2017, 3:56 AM
JonasToth added a comment to D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.

It does still regress! In a sense this is a WIP that i did not clarify.(which i do now)

Dec 4 2017, 3:55 AM

Dec 2 2017

JonasToth added a comment to D40671: [clang-tidy] Support specific checks for NOLINT directive.

I am happy with everything now. But one of the reviewers must accept.

Dec 2 2017, 7:45 AM · Restricted Project
JonasToth added inline comments to D40671: [clang-tidy] Support specific checks for NOLINT directive.
Dec 2 2017, 7:44 AM · Restricted Project
JonasToth added a comment to D40671: [clang-tidy] Support specific checks for NOLINT directive.

Its good that you added code examples to the clang-tidy documentation page. I think that feature was not documented well before.

Dec 2 2017, 5:40 AM · Restricted Project

Dec 1 2017

JonasToth created D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test.
Dec 1 2017, 10:15 AM
JonasToth added inline comments to D40671: [clang-tidy] Support specific checks for NOLINT directive.
Dec 1 2017, 2:11 AM · Restricted Project
JonasToth added inline comments to D40671: [clang-tidy] Support specific checks for NOLINT directive.
Dec 1 2017, 1:26 AM · Restricted Project