[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

Abpostelnicu added a comment. @sammccall was there any progress on this so far? This would be really useful to be a clang-tidy check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121593/new/ https://reviews.llvm.org/D121593

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. In D100581#2798079 , @raj.khem wrote: > http://sprunge.us/FJzZXL is a file from harfbuzz and it warns > > a.cc:28670:32: error: variable 'supp_size' set but not used > [-Werror,-Wunused-but-set-variable] > unsigned

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. I think there is a false positive with this @george.burgess.iv: In [this)(https://searchfox.org/mozilla-central/source/mozglue/baseprofiler/core/platform-linux-android.cpp#222-227) we have the warning triggered:

[PATCH] D100625: [clang-tools-extra] Do not use `LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR`

Abpostelnicu abandoned this revision. Abpostelnicu added a comment. In D100625#2741385 , @hvdijk wrote: > Apparently Phabricator detected the presence of "revert D100625 > " in my alternative D101851 >

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

Abpostelnicu added a comment. I'm seeing here something very strange, if the user provided copy assignment operator is provided but is = delete and the copy constructor is default this warning will still trigger. Is this something expected? I'm referring at this

[PATCH] D101480: Revert "[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable"

Abpostelnicu accepted this revision. Abpostelnicu added a comment. This revision is now accepted and ready to land. Thank you for this! Also I think it’s very well that you want to implement this warning in clang, with a little bit of more polish it will be up for merger! Repository: rG LLVM

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. In D100581#2723611 , @lebedev.ri wrote: > In D100581#2723505 , @Abpostelnicu > wrote: > >> Also I don’t remember seeing this proposed on cfe dev mailing list. > > There is no such

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. Please revert this. This change hasn’t been tested nor reviewed enough. Also I don’t remember seeing this proposed on cfe dev mailing list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

Abpostelnicu added a comment. I think this added a regression, where you have this context: uint32_t LocalAccessible::SelectedItemCount() { uint32_t count = 0; AccIterator iter(this, filters::GetSelected); LocalAccessible* selected = nullptr; while ((selected = iter.Next()))

[PATCH] D93543: clang-tidy: Leave the possibility of opting out having coloured diagnostic messages.

Abpostelnicu updated this revision to Diff 336797. Abpostelnicu added a comment. Add `-use-color` argument to make it sane with `clang-tidy --use-color` argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93543/new/

[PATCH] D93543: clang-tidy: Leave the possibility of opting out having coloured diagnostic messages.

Abpostelnicu created this revision. Abpostelnicu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Afer this commit landed we always have coloured

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

Abpostelnicu added a comment. In D79388#2321231 , @MyDeveloperDay wrote: > Not to my knowledge, if we are not going to fix it we should probably revert > it for now, we can bring it back later I'm a strong supporter of reverting it for the moment.

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

Abpostelnicu added a comment. Has been pushed to the 11.x branch https://github.com/llvm/llvm-project/commit/1596c2dfd548b21cf33ad3353882ac465d78c1bb Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77882/new/ https://reviews.llvm.org/D77882

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

Abpostelnicu added a comment. In D77882#2274292 , @hans wrote: > This seems to have caused https://bugs.llvm.org/show_bug.cgi?id=47512 > > Can you take a look? Are you sure? This has been fixed from august in trunk

[PATCH] D81917: [clang-tidy] For `run-clang-tidy.py` escape the paths that are used for analysis.

Abpostelnicu added a comment. In D81917#2127690 , @njames93 wrote: > In D81917#2127477 , @Abpostelnicu > wrote: > > > @njames93 wdyt if we add another parameter to distinguish if we want to use > > regex or not,

[PATCH] D81917: [clang-tidy] For `run-clang-tidy.py` escape the paths that are used for analysis.

Abpostelnicu added a comment. @njames93 wdyt if we add another parameter to distinguish if we want to use regex or not, and if not we escape the paths? Also thank you so much for catching this up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82784: [clang-tidy] For `run-clang-tidy.py` do not treat `allow_enabling_alpha_checkers` as a none value.

Abpostelnicu created this revision. Abpostelnicu added a reviewer: JonasToth. Herald added subscribers: cfe-commits, Charusso, xazax.hun. Herald added a project: clang. Abpostelnicu edited the summary of this revision. `allow_enabling_alpha_checkers` will never be None, it will be `True` or

[PATCH] D81917: [clang-tidy] For `run-clang-tidy.py` escape the paths that are used for analysis.

Abpostelnicu created this revision. Abpostelnicu added a reviewer: sylvestre.ledru. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Abpostelnicu edited reviewers, added: JonasToth; removed: sylvestre.ledru. Some paths can have special chars like `file++c.cpp` in

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

Abpostelnicu added a comment. Let's first see we don't break anything on `mozilla`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75791/new/ https://reviews.llvm.org/D75791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79990: [clang-format] [PR45614] Incorrectly indents [[nodiscard]] attribute funtions after a macro without semicolon

Abpostelnicu added a comment. Please see https://firefox-source-docs.mozilla.org/code-quality/coding-style/format_cpp_code_with_clang-format.html CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79990/new/ https://reviews.llvm.org/D79990 ___

[PATCH] D79990: [clang-format] [PR45614] Incorrectly indents [[nodiscard]] attribute funtions after a macro without semicolon

Abpostelnicu added a comment. @MyDeveloperDay thanks for the patch, I'm gonna run it agains mozilla to see if there is any fallout. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79990/new/ https://reviews.llvm.org/D79990 ___ cfe-commits

[PATCH] D79201: [clang-format] : Fix additional pointer alignment for overloaded operators

Abpostelnicu accepted this revision. Abpostelnicu added a comment. This revision is now accepted and ready to land. Sorry, totally forgot about this, thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79201/new/ https://reviews.llvm.org/D79201

[PATCH] D79201: [clang-format] : Fix additional pointer alignment for overloaded operators

Abpostelnicu requested changes to this revision. Abpostelnicu added a comment. This revision now requires changes to proceed. As per what @sammccall said. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79201/new/ https://reviews.llvm.org/D79201

[PATCH] D78879: [clang-format] [PR45357] Fix issue found with operator spacing

Abpostelnicu added a comment. Just to clarify something here, for me the patch looks good but until I will accept the revision I want to test it on the mozilla codebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78879/new/ https://reviews.llvm.org/D78879

[PATCH] D78879: [clang-format] [PR45357] Fix issue found with operator spacing

Abpostelnicu added a comment. In D78879#2004856 , @MyDeveloperDay wrote: > @sylvestre.ledru , @Abpostelnicu > > I believe to fix your original request you need a combination of D76850: > clang-format: Fix pointer alignment for overloaded operators

[PATCH] D78879: [clang-format] [PR45357] Fix issue found with operator spacing

Abpostelnicu added a comment. I've cherry-picked D76850 to 10.x and see if this fixes the issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78879/new/ https://reviews.llvm.org/D78879 ___ cfe-commits

[PATCH] D78879: [clang-format] [PR45357] Fix issue found with operator spacing

Abpostelnicu added a comment. In D78879#2004724 , @MyDeveloperDay wrote: > @sylvestre.ledru > > I'm taking a quick look at formatting the original bug in gecko and whilst > the last windows snapshot (Feb 2020) shows the bug > > $ clang-format

[PATCH] D78879: [clang-format] [PR45357] Fix issue found with operator spacing

Abpostelnicu added a comment. I think we are on the right track with this but what if we are dealing with `tok::amp` in a context similar to `operator const FallibleTArray&();` I can speculate that the outcome will be `operator const FallibleTArray &();` which is wrong. CHANGES SINCE LAST

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

Abpostelnicu updated this revision to Diff 256825. Abpostelnicu added a comment. Add release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77882/new/ https://reviews.llvm.org/D77882 Files:

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

Abpostelnicu added a comment. In D77882#1976216 , @sylvestre.ledru wrote: > please add this to the release notes too :) > something like "new option -allow-enabling-alpha-checkers added to > run-clang-tidy to enable alpha checkers" Is it ok just to

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

Abpostelnicu created this revision. Abpostelnicu added a reviewer: JonasToth. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, xazax.hun. Herald added a project: clang. Abpostelnicu edited the summary of this revision.

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

Abpostelnicu added inline comments. Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:88 + + if (!Sources.isInMainFile(ND.getBeginLoc())) +return; This breaks the usage of this checker in an unified build scenario since the main file for the

[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

Abpostelnicu added a comment. done Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74031/new/ https://reviews.llvm.org/D74031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

Abpostelnicu added a comment. In D74031#1859231 , @hans wrote: > Looks great, thanks! > > Do you have commit access? If so, go ahead and push directly to the 10.x > branch, otherwise let me know and I'll do it for you. Yes I have commit access, pushing

[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

Abpostelnicu added a comment. In D74031#1859084 , @hans wrote: > lgtm, but if you want it wouldn't hurt to include even more details from the > commit message. This is a good idea, in my latest patch you will see that I've added more context present

[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

Abpostelnicu updated this revision to Diff 242562. Abpostelnicu added a comment. Adding extra comments to the release patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74031/new/ https://reviews.llvm.org/D74031 Files:

[PATCH] D74031: Update for Clang 10 release notes in order to have reference to D66404.

Abpostelnicu created this revision. Abpostelnicu added a reviewer: hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. Abpostelnicu edited the summary of this revision. Abpostelnicu edited the summary of this revision. Abpostelnicu added a subscriber: sylvestre.ledru.

[PATCH] D72438: [clang-tidy] For checker `readability-misleading-indentation` update tests.

Abpostelnicu added a comment. In D72438#1811512 , @lebedev.ri wrote: > Hm, didn't clang gain such a diagnostic itself recently? > https://godbolt.org/z/MYJTvw > Wouldn't it make sense to migrate everything into it, and drop this > now-duplicating

[PATCH] D72438: [clang-tidy] For checker `readability-misleading-indentation` update tests.

Abpostelnicu created this revision. Abpostelnicu added a reviewer: JonasToth. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Abpostelnicu edited the summary of this revision. Abpostelnicu added a project: clang-tools-extra. In D72333

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

Abpostelnicu added a comment. Will push shortly an update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D72333 ___ cfe-commits mailing list

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

Abpostelnicu added a comment. I think the best approach here is to remove the tests that don’t have template instantiation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D72333

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

Abpostelnicu updated this revision to Diff 236796. Abpostelnicu added a comment. Added more tesst for templa functions that are not instantiated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D72333 Files:

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

Abpostelnicu updated this revision to Diff 236596. Abpostelnicu added a comment. Updated test with match without compound statement. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D72333 Files:

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

Abpostelnicu updated this revision to Diff 236580. Abpostelnicu added a comment. Updated the test case to better reflect the changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72333/new/ https://reviews.llvm.org/D72333 Files:

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

Abpostelnicu created this revision. Abpostelnicu added a reviewer: alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Fixes fixes `readability-misleading-identation` for `if constexpr`. This is very similar to D71980 .

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

Abpostelnicu added a comment. Herald added a project: clang. @aaron.ballman I think the auto usage improves and simplifies the code, however, as I would really like to see this code landed, I would be ok to help Steven to finish this work and replace auto by the "old" way. Do you think we can

[PATCH] D60453: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357994: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number… (authored by Abpostelnicu, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

[PATCH] D60453: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

Abpostelnicu created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D60453 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

This revision was automatically updated to reflect the committed changes. Closed by commit rL342540: [clang-tidy] run-clang-tidy.py - fails using python 3.7 (authored by Abpostelnicu, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

Abpostelnicu updated this revision to Diff 166095. https://reviews.llvm.org/D51220 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py === --- clang-tidy/tool/run-clang-tidy.py +++

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

Abpostelnicu added a comment. I can confirm tested on: 2.7.15 3.7.0 On both it worked. - F7047650: signature.asc https://reviews.llvm.org/D51220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

Abpostelnicu added a comment. In https://reviews.llvm.org/D51220#1212463, @JonasToth wrote: > I don't know a lot about how to write portable code! > > But wouldn't this be out usecase? > http://python-future.org/compatible_idioms.html#file-io-with-open > Using the `decode` is supposed to work

[PATCH] D51220: [clang-tidy] run-clang-tidy fails using python 3.7

Abpostelnicu updated this revision to Diff 162391. https://reviews.llvm.org/D51220 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py === --- clang-tidy/tool/run-clang-tidy.py +++

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu updated this revision to Diff 162387. https://reviews.llvm.org/D49851 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py === --- clang-tidy/tool/run-clang-tidy.py +++

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu added a comment. In https://reviews.llvm.org/D49851#1203676, @JonasToth wrote: > In https://reviews.llvm.org/D49851#1202764, @Abpostelnicu wrote: > > > Strangely I haven't had those kind of issues but since you propose those > > modifications I would do one more modification, let's

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu added a comment. Strangely I haven't had those kind of issues but since you propose those modifications I would do one more modification, let's output everything to stdout and not stderr by doing: if err: sys.stdout.write(str(err) + '\n') Repository: rL LLVM

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

This revision was automatically updated to reflect the committed changes. Closed by commit rL339427: [clang-tidy] run-clang-tidy.py - add synchronisation to the output (authored by Abpostelnicu, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu added a comment. Did you notice any significant speed degradation? https://reviews.llvm.org/D49851 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu marked 2 inline comments as done. Abpostelnicu added a comment. Regarding the time penalty it depends very much on how many files you have. But I would choose anytime to have this in the detriment of having "obfuscated" output. https://reviews.llvm.org/D49851

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu updated this revision to Diff 159511. https://reviews.llvm.org/D49851 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py === --- clang-tidy/tool/run-clang-tidy.py +++

[PATCH] D49851: [clang-tidy] run-clang-tidy add synchronisation to the output

Abpostelnicu created this revision. Abpostelnicu added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. The goal of this patch is to add synchronisation of the output of the tool with the actual files that are verified. Without it the output of clang-tidy will get

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

Abpostelnicu added inline comments. Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:376 +// expression wouldn't really benefit readability. Therefore we abort. +if (NewReturnLength > MaximumLineLength) { + return; lebedev.ri

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

Abpostelnicu added a comment. I can test this on our repo, Mozilla, since it's a large code-base I think we you will have a better understanding of the false-positive ratio. https://reviews.llvm.org/D39121 ___ cfe-commits mailing list

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

This revision was automatically updated to reflect the committed changes. Closed by commit rL297467: [clang-format] Add option to break before inheritance separation operator in… (authored by Abpostelnicu). Changed prior to commit: https://reviews.llvm.org/D30487?vs=91335=91344#toc

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 91335. Abpostelnicu marked an inline comment as done. Abpostelnicu added a comment. Fixed two spell errors. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 91334. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/FormatToken.h

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:355 + if (Current.is(TT_InheritanceColon)) +State.Stack.back().NoLineBreak = true; djasper wrote: > Can you leave a comment here: > > // Don't break within the

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 91330. Abpostelnicu marked an inline comment as done. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 91174. Abpostelnicu added a comment. Yes that's a better name :) Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added a comment. Daniel what do you think about the last version of the patch? I don't want to have this stalled since we really need this modifications in order to be able to run clang-format on our repository. Repository: rL LLVM https://reviews.llvm.org/D30487

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 90480. Abpostelnicu added a comment. corrected some format issues. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 90475. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/FormatToken.h

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added a comment. In https://reviews.llvm.org/D30487#691535, @djasper wrote: > Hm. Unfortunately, this seems to have been implemented to support Webkit > style and Webkit style is explicit about wanting this > (https://webkit.org/code-style-guidelines/) :(. > > But maybe the

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added a comment. In https://reviews.llvm.org/D30487#691517, @djasper wrote: > Do you know whether that is intentional? The style guide isn't really > conclusive. Well i'm not sure, because as you said the document is not clear but i think that when we have a single initialiser

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added a comment. In https://reviews.llvm.org/D30487#691506, @djasper wrote: > Before `'`? Can you give an example? MY mistake, i wanted to write ``:` Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2400 +// Returns 'true' if there is an TT_InheritanceComma in the current token list. +static bool hasMultipleInheritance(const FormatToken ) { + for (const FormatToken* nextTok = Tok.Next; nextTok;

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu added inline comments. Comment at: include/clang/Format/Format.h:309 + /// inheritance. + bool BreakInhertianceBeforeColonAndComma; + djasper wrote: > Hm. I am still not sure about this flag and it's name. Fundamentally, this is > currently

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 90300. Abpostelnicu marked an inline comment as done. Abpostelnicu added a comment. Updated patch with the proposed modifications. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu marked 9 inline comments as done. Abpostelnicu added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2398 +// Determine if the next token from the closing scope is an inheritance token +static bool hasMultipleInheritance(const FormatToken ) {

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 90176. Abpostelnicu added a comment. added patch against the updated repo. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Abpostelnicu updated this revision to Diff 90171. Abpostelnicu added a comment. Also added tests. Repository: rL LLVM https://reviews.llvm.org/D30487 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp