aaronpuchert (Aaron Puchert)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Saturday

  • Clear sailing ahead.

User Details

User Since
Apr 17 2017, 9:32 PM (362 w, 2 d)
Availability
Available

Recent Activity

May 3 2022

aaronpuchert abandoned D22182: Remove anchorClicked lock which causes deadlock.
May 3 2022, 10:16 PM · KDevelop
aaronpuchert commandeered D22182: Remove anchorClicked lock which causes deadlock.
May 3 2022, 10:16 PM · KDevelop

Mar 6 2020

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

As I said, I thought the possibility to cancel jobs could be a comprise (I don't get the impression you've been able to find one yet), but OK.

Mar 6 2020, 10:52 PM · KDevelop

Mar 5 2020

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

I don't want to cancel any parse jobs, I want to change whether and when they emit the preamble.

Mar 5 2020, 11:41 PM · KDevelop

Feb 4 2020

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

Hmm, now I'm thinking: If we parse first without creating a preamble, we still have the Clang AST. Couldn't we create the preamble from the AST without reparsing and use that for subsequent parsing?

Feb 4 2020, 2:45 AM · KDevelop
aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

How bad would it be if we don't do that and only provide KTextEditor-completions before the first edit? I think it's pretty rare that code completion is immediately requested, typically one starts with a line break, or by removing code, before new code is written.

Very bad. I can still remember how bad it felt to use KDevelop before @brauch fixed some nasty issues that prevented us from using the PCH properly. It always felt like KDevelop was utterly broken since it took so long to give us completion answers. And I believe I also remember that quite a few of our users complained about it, and in turn praised the improvement after @brauch fixed it.

You're referring to D6905? That contains a lot of other things. I don't think that adding the flag was the core part of that change, and @brauch doesn't think so either:

Feb 4 2020, 2:41 AM · KDevelop

Feb 2 2020

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

To keep this short I didn't reply to the parts I completely agree with.

Feb 2 2020, 6:23 PM · KDevelop

Jan 14 2020

aaronpuchert added a comment to D25494: Make tar archives reproducible by setting Pax headers.

This should work fine with a producer GNU-tar >= 1.28

We use the option only when SOURCE_DATE_EPOCH is set, and we assume that a recent enough tar is available then. Basically we say that if reproducible builds are desired, the proper tools should be available.

Jan 14 2020, 1:01 AM · KDevelop

Jan 12 2020

aaronpuchert updated subscribers of D25494: Make tar archives reproducible by setting Pax headers.

No insight myself, but would trust you you know what you are doing here :) and it also matches what I just read on https://reproducible-builds.org/docs/archives/

Jan 12 2020, 9:52 PM · KDevelop
aaronpuchert committed R32:25e78ffa2738: Make tar archives reproducible by setting Pax headers (authored by aaronpuchert).
Make tar archives reproducible by setting Pax headers
Jan 12 2020, 9:23 PM
aaronpuchert closed D25494: Make tar archives reproducible by setting Pax headers.
Jan 12 2020, 9:23 PM · KDevelop

Jan 9 2020

aaronpuchert added a reviewer for D25494: Make tar archives reproducible by setting Pax headers: KDevelop.
Jan 9 2020, 12:45 AM · KDevelop

Nov 23 2019

aaronpuchert requested review of D25494: Make tar archives reproducible by setting Pax headers.
Nov 23 2019, 6:04 PM · KDevelop

Oct 9 2019

aaronpuchert added a comment to D22217: Pass extra build flags to compiler for parsing..

I've just stumbled into a case where I would need -stdlib=libc++, which this change would solve.

Oct 9 2019, 9:17 PM · KDevelop
aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

I still think this is the right change, but if there is no consensus I will implement @mwolff's suggestion. That would certainly be an improvement, but it doesn't really solve my problem. I'm currently working on Clang and have around 6 files open, which is really not a lot. I haven't changed any of them. The result:

Oct 9 2019, 9:12 PM · KDevelop

Jul 20 2019

aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

Also, in spite of what I said earlier, I think the explicit DUChain locking should be done and there eventually should be some work done to determine which objects will only interact with one specific DUChain per instance of that object, i.e. an object that should reference a specific DUChain, e.g. ClangSupport. We could make the first pass at this involve replacing all of those DUChainReadLocker lock; lines with DUChainReaderLocker lock(DUChain::lock());, [...]

Jul 20 2019, 8:22 PM · KDevelop
aaronpuchert added a comment to D22217: Pass extra build flags to compiler for parsing..

Overall I think this is a good idea, although we need to figure out some details. Could you either remove commented out flags or add a comment why we have them (commented out)?

Jul 20 2019, 8:18 PM · KDevelop
aaronpuchert committed R32:ad7ea392d9e2: Add working directory to clang parser. (authored by bungeman).
Add working directory to clang parser.
Jul 20 2019, 7:37 PM
aaronpuchert closed D22197: Add working directory to clang parser..
Jul 20 2019, 7:37 PM · KDevelop
aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

I don't know what would be required to support multiple DUChains, but it seems plausible that they can be locked separately. In this case it would help to have the explicit argument, I guess. It could even be inevitable, when there is no single global DUChain anymore.

Jul 20 2019, 7:28 PM · KDevelop
aaronpuchert added a comment to D22559: Bump min dep versions of Qt & KF to 5.9 & 5.44.

I do not think bumping versions "just because " has any value.

Jul 20 2019, 1:06 PM · KDevelop

Jul 10 2019

aaronpuchert accepted D22197: Add working directory to clang parser..
Jul 10 2019, 10:26 PM · KDevelop

Jul 8 2019

aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

The workaround being that we eliminate the try-lock behaviour in our locker constructors and move that timeout behaviour elsewhere?

Exactly.

Jul 8 2019, 11:51 PM · KDevelop

Jul 7 2019

aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

I don't suspect our ..Locker constructors will ever (i.e. not anytime prior to the heat death of the universe) take a different lock. For that reason I am partial to making the lock implicit wherever possible.

That's a bold statement. ;)

Jul 7 2019, 11:42 PM · KDevelop

Jul 4 2019

aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

I have actually tried changing my experiment with clang's thread safety to use the scoping feature and I seem to be seeing a new issue with these destructors with an error like "lock using shared access, expected exclusive access."

EDIT: I have come across your oldish ticket regarding this: https://bugs.llvm.org/show_bug.cgi?id=33504. Evidently I need to annotate my DUChainReadLocker releasing functions with RELEASE(..) instead of RELEASE_SHARED(..). Not sure I understand why.

Jul 4 2019, 6:52 PM · KDevelop
aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

I agree, but in the current implementation they are see DUChainReadLocker::lock() and DUChainReadLocker::unlock(), they both check if we are already locked before continuing to lock or unlock respectively.

Jul 4 2019, 1:42 AM · KDevelop
aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

Interesting. I have experimented with this a bit and I cannot find a way to articulate to the thread safety system that my lock and unlock functions are idempotent. If I call lock.unlock() and later return without locking it again, I get a warning (or rather an error because I set -Werror=thread-safety) that complains that I am releasing a lock that isn't held. So would -Wno-error=thread-safety-negative allow us to get around that or is there possibly some other solution?

Jul 4 2019, 1:25 AM · KDevelop
aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

This should definitely be tried and I would be interested in the task. There is definitely quite alot of stuff that needs to be annotated to make this work so it might need several hands working on it.

Jul 4 2019, 12:48 AM · KDevelop

Jul 3 2019

aaronpuchert added inline comments to D22197: Add working directory to clang parser..
Jul 3 2019, 11:18 PM · KDevelop
aaronpuchert added inline comments to D22197: Add working directory to clang parser..
Jul 3 2019, 12:29 AM · KDevelop
aaronpuchert added a comment to D22182: Remove anchorClicked lock which causes deadlock.

I looked through all of the paths to the failed assertion discussed in that latter ticket and it appears that all but the invocation in PotentialBuddyCollector::accept is verified to have the reader lock held prior to invocation. There were two paths which needed a lock added given my change, so this change set should not cause any regression on bug #386901.

Jul 3 2019, 12:12 AM · KDevelop
aaronpuchert added inline comments to D22217: Pass extra build flags to compiler for parsing..
Jul 3 2019, 12:04 AM · KDevelop

Feb 14 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

Indeed, and in that light clang's solution is maybe not that smart, at least not for use as a parser for an entire project. From what I understand that preamble is just a precompiled header file, which in my experience can be tricky to set up (typically require some sort of central header that imports everything that'll be needed everywhere). Having one such preamble per file cannot but duplicate lots of data.

You're right, it's a precompiled header file. I think it works very well for the intended purpose: quick reparsing on most user edits. The additional option that we use here was added in https://reviews.llvm.org/D15490 for pure code completion workloads. I don't think it's meant for us, even though we do completion as well: we also build an index, and have possibly many more files open.

Feb 14 2019, 11:49 PM · KDevelop
aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

Well, if among developers you cannot find a solution that covers all use cases a configurable setting (applying only to setting the preamble-on-first parse flag or not) could well be the only compromise.

I wouldn't want to give up on a consensus just yet. I think the Clang developers have actually chosen a pretty smart default, and this additional flag incurs costs that are hard to control for hardly any benefit.

Feb 14 2019, 12:40 AM · KDevelop

Feb 13 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

Or, you add a flag to one of the relevant settings panels. There are already a number of options that aren't exactly easy to understand for everyone, this one could be something like "Make code-completion results available faster (can require significant resources)".

Feb 13 2019, 12:04 AM · KDevelop

Feb 11 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

When we didn't create the preamble, we will have to reparse the file completely. And this can easily take 1-2s per file, which is *really* annoying.

It only happens on the very first edit though, and will only be noticeable if code completion is required immediately after starting to edit the file. All following parses and code completions can use the precompiled preamble then. I don't think that's too bad. It's also not unusual, many applications have something like a "warmup phase" where not everything is loaded yet.

Feb 11 2019, 10:48 PM · KDevelop

Feb 6 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

I can't say that it happens systematically but neither that it does NOT happen systematically, and I mean that in general: re-opening a file you just closed.

Isn't this unavoidable BTW, given that we don't end up with preambles of all files?

Feb 6 2019, 12:35 AM · KDevelop

Feb 5 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

preambles are not created during the initial import except possibly for the files that opened with the session if the parser is triggered twice for those.

Feb 5 2019, 1:23 AM · KDevelop

Feb 3 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

If line 3 means "files opened after (1)" then yes, that's probably correct.

Feb 3 2019, 4:07 PM · KDevelop

Feb 2 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

The way I understand it, the proposals are slightly different. Maybe we can summarize them as follows:

Feb 2 2019, 4:13 PM · KDevelop

Feb 1 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

I'm fine with that, but would prefer never having it. Not all files that are opened will be edited, and the delay caused by creating the preamble after the first edit is probably negligible over the lifetime of a session. It would also allow us to just drop affected preambles when a header is edited — otherwise we might have to recreate all preambles that include the header.

Feb 1 2019, 11:18 PM · KDevelop

Jan 31 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

To clarify, the patch back then was less about the first-time delay, and more about the delay being there *every* time because of wrong usage of the API. Whether the delay occurs once is arguably a bit less important.

Jan 31 2019, 12:37 AM · KDevelop

Jan 29 2019

aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

Aaron, don't your arguments apply to parsing an entire project on startup as well? I can easily imagine that opening and parsing entire projects of the size you mention must have a non-negligible cost regardless of all optimisation and caching that is currently implemented

It definitely does. Sometimes I just cancel it, but sometimes I need the entire index to understand some code. (“Find uses” is very useful for that.)

Jan 29 2019, 10:06 PM · KDevelop
aaronpuchert added a comment to D18551: clang: Create preamble only on second parse.

Preambles are there to speed up repeated reparsing. Most translation units won't be reparsed in a typical session. Since we pay for every preamble we create, I don't think we should create them unless we've some indication that a file needs to be parsed again. It's pretty safe to assume that a file needs to be reparsed again if we parse it for the second time. There is probably no better indicator, which is why this is the default.

Jan 29 2019, 1:16 AM · KDevelop

Jan 26 2019

aaronpuchert added a comment to D17618: Fix override function signature of createCompletionContext().

Wouldn't that be an opportunity to add the override specifier? The commit message could state "Fix function signature of ... to override".

Jan 26 2019, 11:35 PM · KDevelop
aaronpuchert added a comment to D17289: KDevelop/Shell: set dedicated TMPDIR.

I've tried to unlink certain files after they were created but reverted that again when I started getting crashes in the parser. Not that reverting helped against that...

Yes, I believe that's not going to work. Clang doesn't keep the preamble files open, but expects them to stay.

Jan 26 2019, 9:15 PM · KDevelop
aaronpuchert requested review of D18551: clang: Create preamble only on second parse.
Jan 26 2019, 3:33 PM · KDevelop
aaronpuchert added a comment to D17289: KDevelop/Shell: set dedicated TMPDIR.

I've had a look at the Clang code. They actually go to some lengths to make sure all temporary files are deleted on a clean exit — they register all open temporary files and clean them up in an exit-time destructor. (See TemporaryFiles in lib/Frontend/PrecompiledPreamble.cpp.) However, this won't work in the case of a crash. They have some mechanisms for cleanup in the case of crashes, but I suspect that this works by setting a signal handler and is thus only available in the stand-alone executable. There are mechanisms to ensure cleanup in any event by removing an open file on POSIX compatibles (close will then automatically delete it), or FILE_FLAG_DELETE_ON_CLOSE on Windows. But that doesn't work for Clang, because they work with paths and don't pass file descriptors around.

Jan 26 2019, 2:45 PM · KDevelop

Jan 25 2019

aaronpuchert added a comment to D18224: Clang Plugin: Report some problems from included files.

To some extent that problem is going to disappear as soon as concepts are more widely used. GCC ≥ 6 already supports them and I expect Clang to follow suit — there is already a prototype.

Jan 25 2019, 2:07 AM · KDevelop

Jan 24 2019

aaronpuchert added a comment to D17289: KDevelop/Shell: set dedicated TMPDIR.

If you don't use systemd, for example because you're not on Linux, there are certainly other tools for doing the same thing.

How does that systemd thing clean tmp dirs at runtime, IOW, how can it know it's safe to clean up a given file if the application that created it doesn't do something explicit to guarantee cleanup?

Jan 24 2019, 11:23 PM · KDevelop

Jan 23 2019

aaronpuchert added a comment to D17289: KDevelop/Shell: set dedicated TMPDIR.

Temporary files being left behind after a crash is not unusual. This is why systemd has systemd-tmpfiles --clean.

Not really relevant outside of the systemd realm, right? :)

Jan 23 2019, 11:01 PM · KDevelop

Jan 17 2019

aaronpuchert added a comment to D17289: KDevelop/Shell: set dedicated TMPDIR.

Actually, issues with clang temp files is why I started to think about this kind of change but ultimately I realised it didn't seem such a bad idea at all to put all KDevelop-related temporary files in a dedicated location. I often clean out a bunch of KDevelop's own temp files that were left behind, e.g. after a crash. I just didn't mention them because they're negligible in size (and I purge before their numbers really start to grow).

Jan 17 2019, 10:11 PM · KDevelop

Dec 10 2018

aaronpuchert added inline comments to D16484: Add scratchpad plugin.
Dec 10 2018, 9:03 PM · KDevelop

Nov 18 2018

aaronpuchert added inline comments to D16218: [KDevelop/Core]: safe signal-handler implementation.
Nov 18 2018, 9:30 PM · KDevelop

Nov 14 2018

aaronpuchert added a comment to D16218: [KDevelop/Core]: safe signal-handler implementation.

Maybe you can add an else branch where you re-raise the signal with the default handler.

If the goal is just to let a 2nd signal trigger the default reaction from the runtime then it can be even simpler.

Nov 14 2018, 12:10 AM · KDevelop

Oct 20 2018

aaronpuchert added a comment to D16338: Improve R documentation highlighting.

Thanks for the quick review!

Oct 20 2018, 6:36 PM · Frameworks, Kate
aaronpuchert committed R216:445a3e9ccf52: Improve R documentation highlighting (authored by aaronpuchert).
Improve R documentation highlighting
Oct 20 2018, 6:34 PM
aaronpuchert closed D16338: Improve R documentation highlighting.
Oct 20 2018, 6:34 PM · Frameworks, Kate
aaronpuchert committed R216:dd52b4cd3dd2: No spell checking for Metamath except in comments (authored by aaronpuchert).
No spell checking for Metamath except in comments
Oct 20 2018, 5:04 PM
aaronpuchert requested review of D16338: Improve R documentation highlighting.
Oct 20 2018, 4:38 PM · Frameworks, Kate
aaronpuchert accepted D16218: [KDevelop/Core]: safe signal-handler implementation.

The signal handling and processing gets a +1 from me, but someone else should review the overall code structure.

Oct 20 2018, 2:04 PM · KDevelop

Oct 19 2018

aaronpuchert added inline comments to D16218: [KDevelop/Core]: safe signal-handler implementation.
Oct 19 2018, 8:45 PM · KDevelop

Oct 18 2018

aaronpuchert added a comment to D16218: [KDevelop/Core]: safe signal-handler implementation.

So is it enough to change to std::atomic_flag or should we use some sort of critical section with an actual lock? Because for handlingSignal that's possible as it is outside of the actual signal handler.

Oct 18 2018, 8:08 PM · KDevelop

Oct 17 2018

aaronpuchert added a comment to D16218: [KDevelop/Core]: safe signal-handler implementation.

At least std::atomic<bool> is lock-free on all relevant architectures. (I'm aware that the standard doesn't guarantee it, probably because of some embedded architectures.

But is it always lock-free on the relevant architectures? Or rather:

  • what determines if the type is lock-free
  • what are "the relevant architectures"?
Oct 17 2018, 11:55 PM · KDevelop

Oct 16 2018

aaronpuchert added a comment to D16218: [KDevelop/Core]: safe signal-handler implementation.

You're right, my remarks apply to the existing code already.

Oct 16 2018, 8:33 PM · KDevelop

Oct 15 2018

aaronpuchert added inline comments to D16218: [KDevelop/Core]: safe signal-handler implementation.
Oct 15 2018, 10:28 PM · KDevelop

Oct 4 2018

aaronpuchert added a comment to D15955: clang: Also detect Clang builtin dirs at runtime.

Don't we know the exact version of Clang at compile-time when building the bundled version? I think this change is mainly interesting for the "unbundled" case when Clang can be updated independently. Or what am I missing?

Oct 4 2018, 10:31 PM · KDevelop

Oct 3 2018

aaronpuchert added a comment to D15895: Use CLANG_INCLUDE_DIRS for clang include dir.

"cpuid.h" on my system is in:

  • /usr/lib64/clang/7.0.0/include/cpuid.h

$ llvm-config --libdir
/usr/lib64/llvm/7/lib64

Oct 3 2018, 9:32 PM · KDevelop

Oct 2 2018

aaronpuchert committed R32:3a2f9a9d9b86: Make implicit fallthroughs a compiler error, remove unneeded breaks (authored by aaronpuchert).
Make implicit fallthroughs a compiler error, remove unneeded breaks
Oct 2 2018, 9:28 PM
aaronpuchert closed D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.
Oct 2 2018, 9:28 PM · KDevelop
aaronpuchert committed R32:6b8912678e2f: Add missing break in QmlJs code completion (authored by aaronpuchert).
Add missing break in QmlJs code completion
Oct 2 2018, 9:28 PM
aaronpuchert edited reviewers for D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks, added: kossebau; removed: mssola, brauch.

@kossebau Are you Ok with this change? I'm not sure I'll get a review from the domain experts, but I'm fairly confident now that the changes are right.

Oct 2 2018, 3:35 PM · KDevelop
aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

My point here was that making a whatever-you-call it more or less obligatory in something that is (used to be) shared with a closely related language introduces unnecessary confusion ("wait, am I writing C or C++ here"). And extra overhead when reusing algorithms between those languages.

Oct 2 2018, 3:33 PM · KDevelop

Sep 27 2018

aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

Attributes are not keywords — they're a (C++-specific) generic syntax that was introduced with C++11. There are numerous compiler-specific attributes, usually prefixed with the compiler name, and some standardized attributes. See cppreference.com for a list of those.

Sep 27 2018, 11:49 PM · KDevelop
aaronpuchert added reviewers for D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks: mssola, brauch.
Sep 27 2018, 5:58 PM · KDevelop
aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

My gripe is more with having to change my habits in ways I don't necessarily agree with entirely, and which might interfere with coding in other contexts.
It may not in KDevelop's code, but that's not all that all of us work on. Coding style and guidelines for KDevelop itself could reflect good practice you'd like to advocate in general.

Sep 27 2018, 5:55 PM · KDevelop
aaronpuchert accepted D15764: [Custom-DefinesAndIncludes]: Objective-C++ support.
Sep 27 2018, 5:46 PM · KDevelop
aaronpuchert committed R32:0fddfa4f69d6: Document language type argument in ICompiler interface (authored by aaronpuchert).
Document language type argument in ICompiler interface
Sep 27 2018, 4:35 PM
aaronpuchert added a comment to D15764: [Custom-DefinesAndIncludes]: Objective-C++ support.

Given that languageOption also has Q_UNREACHABLE(), and both are always called together, any crash that could happen would have already happened before. The function is called from CompilerProvider::{defines,includes} and both catch the Other case before calling into the ICompiler implementation. Maybe we should document the constraint (type != Utils::Other) in the ICompiler interface, but we should do it in a separate commit.

Sep 27 2018, 4:31 PM · KDevelop

Sep 26 2018

aaronpuchert accepted D15764: [Custom-DefinesAndIncludes]: Objective-C++ support.

I didn't test it, but it looks fine to me.

Sep 26 2018, 11:43 PM · KDevelop
aaronpuchert added inline comments to D15764: [Custom-DefinesAndIncludes]: Objective-C++ support.
Sep 26 2018, 4:16 PM · KDevelop
aaronpuchert added a comment to D15764: [Custom-DefinesAndIncludes]: Objective-C++ support.

So I think there are two questions to decide here:

  • Do we want a -std= flag, does Clang even accept that?
  • Should we just take the ObjC flags from the C flags, and ObjC++ flags from the C++ flags, or should they be set independently?
Sep 26 2018, 2:33 PM · KDevelop

Sep 23 2018

aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

After testing the GitHub provider and the QmlJS code completion I'm somewhat confident that these changes are correct, but I want to give the experts on that code a chance to weigh in before I merge this. Now onto the discussion.

Sep 23 2018, 9:21 PM · KDevelop
aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

As mentioned on another ticket, I don't like the idea of not putting breaks, no matter how clever the compiler and hard-coded choice of compiler options.

Sep 23 2018, 6:04 PM · KDevelop
aaronpuchert updated the diff for D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

Remove comment in plugins/ghprovider/ghproviderwidget.cpp, as the fallthrough is probably intended, and add break in plugins/qmljs/codecompletion/context.cpp, as it is probably not intended. Now the code completion doesn't suggest variables of the same type as the last parameter in functions calls, which seems about right.

Sep 23 2018, 4:58 PM · KDevelop
aaronpuchert updated subscribers of D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.
Sep 23 2018, 4:42 PM · KDevelop
aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

Yes, needs to be included for such source file, because the min Qt dependency version is still 5.5, and Q_FALLTHROUGH was only introduced in 5.8 (see note at http://doc.qt.io/qt-5/qtglobal.html#Q_FALLTHROUGH)

Sep 23 2018, 2:24 PM · KDevelop

Sep 22 2018

aaronpuchert added a comment to D15532: [Astyle] Add Objective C to list of languages with formatters.

I have no (big) problems with a fallthrough attribute, but if it's going to be obligatory I'd want the complementary attribute (the break) to be obligatory too...

Sep 22 2018, 8:39 PM · KDevelop
aaronpuchert updated subscribers of D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

@kossebau It seems you did something similar in D6301, maybe you can review this change. Should I also include qtcompat_p.h when Q_FALLTHROUGH is used?

Sep 22 2018, 5:06 PM · KDevelop
aaronpuchert added a comment to D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.

There are two suspicious fallthroughs, maybe someone with a better understanding than me can comment on that.

Sep 22 2018, 4:52 PM · KDevelop
aaronpuchert requested review of D15694: Make implicit fallthroughs a compiler error, remove unneeded breaks.
Sep 22 2018, 4:50 PM · KDevelop
aaronpuchert added a comment to D15532: [Astyle] Add Objective C to list of languages with formatters.

I prefer to remind myself of such things and that I'm putting it there anyway (because the compiler won't be able to tell me I really forgot to use one, not unless you make some kind of "no break here" statement obligatory).

Sep 22 2018, 2:37 PM · KDevelop

Sep 21 2018

aaronpuchert added a comment to D15530: kdev-clang : somewhat more complete ObjC(++) support.

You should probably also add a few lines to the custom-definesandincludes plugin.

In a different review I presume?

Sep 21 2018, 9:49 PM · KDevelop
aaronpuchert added a comment to D15532: [Astyle] Add Objective C to list of languages with formatters.

We could activate -Wunreachable-code so the compiler tells you where break isn't needed.

Evidently I was talking about a general principle, not about introducing a dependency on the compiler telling you what (not) to do. If I wanted that I'd be coding in Modula-2 ...

Sep 21 2018, 9:45 PM · KDevelop
aaronpuchert added a comment to D15530: kdev-clang : somewhat more complete ObjC(++) support.

You should probably also add a few lines to the custom-definesandincludes plugin.

Sep 21 2018, 12:32 AM · KDevelop
aaronpuchert added inline comments to D15532: [Astyle] Add Objective C to list of languages with formatters.
Sep 21 2018, 12:09 AM · KDevelop

Aug 24 2018

aaronpuchert added a comment to D14931: Eliminate duplicate QMaps in OutputWidget.

I was in doubt between these two variants while writing the code and stopped on current one for two reasons:

  • Looking at QT source code, reset() is ultimately reduced to QSharedPointer copy(t); swap(copy); anyways. And QSharedPointer has move constructor, so it'll work in this case (right?).
  • Current variant better expresses my intentions (that's subjective). But if you still think that we should use reset() I'd change it in a moment. What do you think?
Aug 24 2018, 8:46 PM · KDevelop

Aug 23 2018

aaronpuchert added inline comments to D14931: Eliminate duplicate QMaps in OutputWidget.
Aug 23 2018, 10:55 PM · KDevelop

Aug 13 2018

aaronpuchert committed R32:248e34d1ece0: Store sizeof and friends as numbers instead of comment. (authored by michalsrb).
Store sizeof and friends as numbers instead of comment.
Aug 13 2018, 8:18 PM
aaronpuchert closed D14479: Store sizeof and friends as numbers instead of comment..
Aug 13 2018, 8:18 PM · KDevelop