[PATCH] D45059: Add a clang-tidy check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-29 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv created this revision. george.burgess.iv added reviewers: aaron.ballman, alexfh. Herald added a subscriber: mgorny. `TEMP_FAILURE_RETRY(x)` is a macro that executes `x` until `(x) != -1 || errno != EINTR`, and evaluates to the result of the last execution of `x`. I've been told

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-29 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 140324. george.burgess.iv marked 3 inline comments as done. george.burgess.iv edited the summary of this revision. george.burgess.iv added a comment. Addressed feedback. https://reviews.llvm.org/D45059 Files: clang-tidy/bugprone/BugproneTidyModu

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-29 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks! > Will be good idea to clarify where TEMP_FAILURE_RETRY come from. Updated the CL summary and added "TEMP_FAILURE_RETRY is a macro provided by both glibc and Bionic." to ComparisonInTempFailureRetryCheck.h. Did you have anything else in mind? =

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 140459. george.burgess.iv marked an inline comment as done. george.burgess.iv added a comment. Addressed feedback https://reviews.llvm.org/D45059 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for the feedback! > You noted, that the C++ warning would catch this case, but does not get > triggered in C. Would it be wort to generalize the concern and have a warning > that catch the comparison, regardless of the macro? I'm not quite sure how we

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. > C89 has no bool type and no stdbool.h but it has been added to later > standards? That means the generalization could theoretically be done for > later C standards, because we could check if the bool typedef had been used? > If yes, would the check benefit?

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-02 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 140675. george.burgess.iv marked 5 inline comments as done. george.burgess.iv added a comment. Addressed feedback https://reviews.llvm.org/D45059 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-02 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments. Comment at: clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.cpp:78 + + diag(RHS.getOperatorLoc(), + "Top-level comparisons should be moved out of TEMP_FAILURE_RETRY"); JonasToth wrote: > You could even provide

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-09 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for the feedback! > and I suspect that your interest in this check is also related to android? Yup :) > Alternatively, if there are other checks specific to the GNU C library > planned, we could add a new module for it. I have nothing planned, so I'm

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-09 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 141687. george.burgess.iv marked 2 inline comments as done. george.burgess.iv added a comment. Herald added a subscriber: srhines. Addressed feedback https://reviews.llvm.org/D45059 Files: clang-tidy/android/AndroidTidyModule.cpp clang-tidy/an

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

2018-04-09 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 141693. george.burgess.iv added a comment. Rebased https://reviews.llvm.org/D38479 Files: docs/UsersManual.rst include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/LangOptions.def include/clang/Driver/CC1Options.td include/clan

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

2018-04-09 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Hi! It fell off my radar, but I'm happy to put it back on my queue. :) There's still a few aarch64-specific backend bits I need to fix before this patch should go in. https://reviews.llvm.org/D38479 ___ cfe-commi

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-09 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks! I plan to commit this tomorrow, to give time for any last-minute comments. Thanks again to everyone for the review. :) https://reviews.llvm.org/D45059 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-09 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 141768. george.burgess.iv marked 2 inline comments as done. george.burgess.iv added a comment. Address feedback https://reviews.llvm.org/D45059 Files: clang-tidy/android/AndroidTidyModule.cpp clang-tidy/android/CMakeLists.txt clang-tidy/andr

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-04-10 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329759: [clang-tidy] Add a `android-comparison-in-temp-failure-retry` check (authored by gbiv, committed by ). Herald added subscribers: llvm-commits, klimek. Changed prior to commit: https://reviews.l

[PATCH] D14274: Add alloc_size attribute to clang

2016-12-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv marked an inline comment as done. george.burgess.iv added a comment. Thanks, everyone! :) Repository: rL LLVM https://reviews.llvm.org/D14274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D14274: Add alloc_size attribute to clang

2016-12-19 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL290149: Add the alloc_size attribute to clang. (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D14274?vs=77222&id=82046#toc Repository: rL LLVM https://reviews.llvm.org/D14274

[PATCH] D26410: [CodeGen] Don't emit the same global block multiple times.

2016-12-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv abandoned this revision. george.burgess.iv added a comment. Committed in https://reviews.llvm.org/rL290149. Thanks again! https://reviews.llvm.org/D26410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D27424: Add the diagnose_if attribute to clang.

2017-01-04 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 83188. george.burgess.iv marked 18 inline comments as done. george.burgess.iv added a comment. Addressed all feedback + made `diagnose_if` late-parsed. https://reviews.llvm.org/D27424 Files: include/clang/AST/Expr.h include/clang/Basic/Attr.td

[PATCH] D27424: Add the diagnose_if attribute to clang.

2017-01-04 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Do we have a page that details when we should/shouldn't use `auto`? I was under the impression that it was preferred only in cases where the type's spelled out (e.g. `cast`, ...). (To be clear, I'm happy to use it in loops, too; I'd just like to know if we hav

[PATCH] D27424: Add the diagnose_if attribute to clang.

2017-01-06 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 83412. george.burgess.iv marked 2 inline comments as done. george.burgess.iv added a comment. Addressed feedback. Thanks! https://reviews.llvm.org/D27424 Files: include/clang/AST/Expr.h include/clang/Basic/Attr.td include/clang/Basic/AttrDoc

[PATCH] D27424: Add the diagnose_if attribute to clang.

2017-01-06 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. > We do: > http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable Awesome. Thanks! > Btw, there's a question in there about late parsing that Phab won't let me > delete, feel free to ignore it. ;-) OK. The answer is "yes

[PATCH] D27424: Add the diagnose_if attribute to clang.

2017-01-08 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL291418: Add the diagnose_if attribute to clang. (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D27424?vs=83412&id=83584#toc Repository: rL LLVM https://reviews.llvm.org/D27424

[PATCH] D27424: Add the diagnose_if attribute to clang.

2017-01-08 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for the review! :) Repository: rL LLVM https://reviews.llvm.org/D27424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30760: Record command lines in objects built by clang

2017-03-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. This is looking pretty good! Aside from a few nits, I'm happy with this after we add tests. Comment at: lib/Driver/ToolChains/Clang.cpp:2725 + if (Args.hasArg(options::OPT_grecord_gcc_switches)) { +std::string CmdLineOpts = ""; +for

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-23 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:4328 + if (getToolChain().UseDwarfDebugFlags() || + Args.hasArg(options::OPT_grecord_gcc_switches)) { ArgStringList OriginalArgs; looks like we have to consider `-gno-

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-24 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments. Comment at: test/Driver/debug-options.c:201 // +// GRECORD: "-dwarf-debug-flags" +// GRECORD: -### -c -grecord-gcc-switches echristo wrote: > This seems a little light on the testing, would you mind adding some more > in

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-18 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv created this revision. As it turns out, emitting diagnostics from places where you're not meant to emit them from is a very bad idea. :) After some looking around, it seems that it's less insane to check for `diagnose_if` attributes in code that's already checking for e.g. nul

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 84988. george.burgess.iv added a comment. Sprinkle in a few `const`s, use `const auto *` in range for loops. https://reviews.llvm.org/D28889 Files: include/clang/Sema/Overload.h include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sema/

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-23 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 85486. george.burgess.iv marked 2 inline comments as done. george.burgess.iv added a comment. Addressed all feedback. Richard noted that, because we're now doing these checks after overload resolution has occurred, we no longer need to convert argu

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-26 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Pinging early because this is a release blocker. :) https://reviews.llvm.org/D28889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for the feedback! Comment at: lib/Sema/SemaChecking.cpp:2520 + +// TODO: Call can technically be a const CallExpr, but const_casting feels ugly, +// and I really don't want to duplicate unwrapCallExpr's logic. No caller really ---

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 86111. george.burgess.iv marked 2 inline comments as done. george.burgess.iv added a comment. Address feedback https://reviews.llvm.org/D28889 Files: include/clang/Sema/Overload.h include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp lib/Sem

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 86146. george.burgess.iv marked 7 inline comments as done. george.burgess.iv added a comment. Addressed all feedback > Another "fun" testcase Ooh, shiny. Added. https://reviews.llvm.org/D28889 Files: include/clang/Sema/Overload.h include/cla

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments. Comment at: test/SemaCXX/diagnose_if.cpp:615 +// evaluator isn't able to evaluate `adl::Foo(1)` to a constexpr, though. +// I'm assuming this is because we assign it to a temporary. +for (void *p : adl::Foo(1)) {} -

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments. Comment at: lib/Sema/SemaChecking.cpp:11933 } - aaron.ballman wrote: > Unintended change? ...I dunno what keeps making this change, but I'll re-undo it before I submit. https://reviews.llvm.org/D28889 ___

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL293360: Change how we handle diagnose_if attributes. (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D28889?vs=86146&id=86155#toc Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D28889: Change where we handle arg-dependent diagnose_if attributes

2017-01-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for the review! Repository: rL LLVM https://reviews.llvm.org/D28889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29469: Fix PR31843: Clang-4.0 crashes/assert while evaluating __builtin_object_size

2017-02-02 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv created this revision. We're currently using a special EvaluationMode to determine whether we're OK with invalid base expressions during objectsize evaluation. Using it to figure out how we handle UB/etc. is fine, but I think it's too far-reaching to use for checking whether we'r

[PATCH] D29773: Add support for armv7ve flag in clang (PR31358).

2017-02-09 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL294662: Add support for armv7ve flag in clang (PR31358). (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D29773?vs=87831&id=87899#toc Repository: rL LLVM https://reviews.llvm.o

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-06-27 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Woohoo! Thanks again to both of you. https://reviews.llvm.org/D32332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-06-27 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL306467: [Sema] Allow unmarked overloadable functions. (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D32332?vs=103448&id=104277#toc Repository: rL LLVM https://reviews.llvm.or

[PATCH] D33904: Add a __has_attribute_enhancement macro to clang

2017-07-11 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv abandoned this revision. george.burgess.iv added a comment. r306467 used `__has_extension(overloadable_unmarked)` instead. Thanks again for the comments! https://reviews.llvm.org/D33904 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D35338: Add the -fdestroy-globals flag

2017-07-18 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for working on this! One small drive-by nit for you. Same "no need to update the diff this very second" as vsk; I can't LGTM this with confidence. (Also, in the future, please try to include context

[PATCH] D29469: Fix PR31843: Clang-4.0 crashes/assert while evaluating __builtin_object_size

2017-02-10 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. End of week ping :) https://reviews.llvm.org/D29469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29469: Fix PR31843: Clang-4.0 crashes/assert while evaluating __builtin_object_size

2017-02-10 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv marked an inline comment as done. george.burgess.iv added a comment. Thanks! https://reviews.llvm.org/D29469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29469: Fix PR31843: Clang-4.0 crashes/assert while evaluating __builtin_object_size

2017-02-10 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL294800: Don't let EvaluationModes dictate whether an invalid base is OK (authored by gbiv). Changed prior to commit: https://reviews.llvm.org/D29469?vs=86870&id=88061#toc Repository: rL LLVM https:/

[PATCH] D14274: Add alloc_size attribute to clang

2016-11-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Ping https://reviews.llvm.org/D14274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27424: Add the diagnose_if attribute to clang.

2016-12-05 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv created this revision. george.burgess.iv added reviewers: rsmith, aaron.ballman. george.burgess.iv added subscribers: EricWF, cfe-commits. This patch adds the `diagnose_if` attribute to clang. `diagnose_if` is meant to be a less powerful version of `enable_if`: it doesn't inter

[PATCH] D14274: Add alloc_size attribute to clang

2016-12-05 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Ping :) https://reviews.llvm.org/D14274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27424: Add the diagnose_if attribute to clang.

2016-12-12 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Glad you like it! > The second thing that would be amazing if this attribute was late parsed so > it the ability to reference this when applied to member functions That seems like a good idea; I'll look into what that would take. (In the meantime, reviews wou

[PATCH] D27424: Add the diagnose_if attribute to clang.

2016-12-13 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 81259. george.burgess.iv marked an inline comment as done. george.burgess.iv added a comment. Addressed alignment comment. Thanks! (Late parsing is still to-be-done) https://reviews.llvm.org/D27424 Files: include/clang/Basic/Attr.td include/cl

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-04-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv created this revision. One of the common use-cases for the `overloadable` attribute is to create small wrapper functions around an existing function that was previously not overloadable. For example: // C Library v1 void foo(int i); // C Library v2 // Note the __as

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-01 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. ping :) https://reviews.llvm.org/D32332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-01 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. thanks for the feedback! fwiw, at a high level, the main problem i'm trying to solve with this is that you can't really make a `__overloadable_without_mangling` macro without handing it the function name, since you currently need to use `__asm__("name-you'd-l

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-03 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. > I'd like to understand this use case a little bit better. If you don't > perform the mangling, then choosing which overload gets called is already > based on the source order, isn't it? See https://godbolt.org/g/8b10Ns I think I might have miscommunicated: `

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-03 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 97698. george.burgess.iv added a comment. We now require `transparently_overloadable` on all redeclarations. Allowing mixed `transparently_overloadable` and `overloadable` coming soon. https://reviews.llvm.org/D32332 Files: include/clang/Basic/

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-10 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments. Comment at: lib/Sema/SemaDecl.cpp:2921 + const auto *NewOvl = New->getAttr(); + if (NewOvl->isTransparent() != OldOvl->isTransparent()) { +assert(!NewOvl->isImplicit() && aaron.ballman wrote: > Can `NewOv

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-10 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 98569. george.burgess.iv marked 7 inline comments as done. george.burgess.iv added a comment. - Addressed all feedback - Now we emit a warning when `transparently_overloadable` "overrides" `overloadable`, rather than an error https://reviews.llvm.

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-10 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. I'd be happy with that approach. Do you like it, Aaron? FWIW, I did a bit of archaeology, and it looks like the commit that added the requirement that all overloads must have `overloadable` (r64414) did so to keep users from "trying to be too sneaky for their

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-23 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 99851. george.burgess.iv added a comment. Remove the `transparent_overloadable` attribute entirely. This approach presents one problem that I didn't see until I implemented it: I'd like to have something to detect that this feature exists. The quic

[PATCH] D32332: Add support for transparent overloadable functions in clang

2017-05-23 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Shortly after I pressed submit, I realized that this patch allows the following code if you tweak an assert to check for `overloadable` on the most recent redecl of a function: void foo(int); void foo(int) __attribute__((overloadable)); void foo(float);

[PATCH] D99993: bugprone-argument-comment: ignore name mismatches for decls from system headers

2021-04-06 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv created this revision. george.burgess.iv added reviewers: hokein, gribozavr2. george.burgess.iv added a project: clang. Herald added a subscriber: jfb. george.burgess.iv requested review of this revision. Herald added a project: clang-tools-extra. As of 2a3498e24f97d

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

2021-04-06 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for the comment! > Why `--header-filter` command line option or `HeaderFilterRegex` > configuration file option could not solve this problem? AFAICT, `--header-filter` & `HeaderFilterRegex` exist to filter diagnostics that occur in matching headers. Th

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

2021-04-15 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for this! I think this warning looks valuable. Most of my comments are various forms of style nits. :) Comment at: clang/lib/Sema/SemaDecl.cpp:13738 +// values in Map should be true. traverses Body; if any key is used in any way +//

[PATCH] D99993: [clang-tidy] bugprone-argument-comment: ignore name mismatches for decls from system headers

2021-04-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. friendly ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D3/new/ https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

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

2021-04-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a reviewer: rtrieu. george.burgess.iv added a comment. Just a few more nits and LGTM. We probably want the thoughts of someone with ownership in warnings to be sure. +rtrieu might be good? Comment at: clang/lib/Sema/SemaDecl.cpp:13740 +// other than ass

[PATCH] D92892: [clang] Change builtin object size to be compatible with GCC when sub-object is invalid

2021-01-06 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. thanks for working on this! just one tiny nit and lgtm Comment at: clang/lib/AST/ExprConstant.cpp:11408 // If we point to before the start of the

[PATCH] D92892: [clang] Change builtin object size to be compatible with GCC when sub-object is invalid

2021-01-07 Thread George Burgess IV via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG275f30df8ad6: [clang] Change builtin object size when subobject is invalid (authored by jtmott-intel, committed by george.burgess.iv). Herald added a project: clang. Repository: rG LLVM Github Monorepo

[PATCH] D92892: [clang] Change builtin object size to be compatible with GCC when sub-object is invalid

2021-01-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. reverted in https://github.com/llvm/llvm-project/commit/b270fd59f0a86fe737853abc43e76b9d29a67eea until we can figure out how to address the issues outlined above. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

2021-10-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv accepted this revision. george.burgess.iv added a comment. This revision is now accepted and ready to land. LGTM % nits -- thanks for this! :) Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:836 +def warn_fortify_scanf_overflow : Warning < + "'

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

2021-10-19 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment. Thanks for this! The idea LGTM, and sounds like a solid way for us to do better about diagnosing FORTIFY'ed calls in Clang. I have a handful of mostly nits/questions for you :) Comment at: clang/include/clang/Basic/Attr.td:3822 +def Diagnose

[PATCH] D111833: [clang] Fortify warning for scanf calls with field width too big.

2021-10-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv accepted this revision. george.burgess.iv added a comment. LGTM. Thanks again! Comment at: clang/lib/Sema/SemaChecking.cpp:735 + +auto *FormatExpr = TheCall->getArg(FormatIndex)->IgnoreParenImpCasts(); + nit: const auto if possible (and be

<    1   2