This revision was automatically updated to reflect the committed changes.
Closed by commit rL294402: [AVR] Add support for the 'interrupt' and 'naked'
attributes (authored by dylanmckay).
Changed prior to commit:
https://reviews.llvm.org/D28451?vs=87365=87588#toc
Repository:
rL LLVM
ehsan created this revision.
Herald added a subscriber: JDevlieghere.
These flags allow specifying extra arguments to the tool's command
line which don't appear in the compilation database.
https://reviews.llvm.org/D29699
Files:
clang-tidy/tool/clang-tidy-diff.py
Index:
jbangert marked 5 inline comments as done.
jbangert added a comment.
Thank you for the initial feedback!
Comment at: include/clang/Tooling/RefactoringCallbacks.h:61
+MatchFinder.addMatcher(Matcher, Callback);
+Callbacks.emplace_back(Callback);
+ }
jbangert marked 2 inline comments as done.
jbangert added a comment.
(marking other comments as done).
https://reviews.llvm.org/D29621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
pelikan created this revision.
Functions with the "xray_log_args" attribute will tell LLVM to emit a special
XRay sled for compiler-rt to copy any call arguments to your logging handler.
https://reviews.llvm.org/D29704
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
ehsan added a comment.
This is equivalent to https://reviews.llvm.org/D28334 for clang-tidy-diff.py.
https://reviews.llvm.org/D29699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jbangert updated this revision to Diff 87574.
jbangert added a comment.
Address code feedback & Reformat with clang-format --style llvm.
https://reviews.llvm.org/D29621
Files:
include/clang/Tooling/RefactoringCallbacks.h
lib/Tooling/RefactoringCallbacks.cpp
jbangert updated this revision to Diff 87575.
jbangert added a comment.
- change to push_back
https://reviews.llvm.org/D29621
Files:
include/clang/Tooling/RefactoringCallbacks.h
lib/Tooling/RefactoringCallbacks.cpp
unittests/Tooling/RefactoringCallbacksTest.cpp
Index:
Eugene.Zelenko added a comment.
Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).
This check is also should be included into cppcoreguidelines module. Or may be
moved there?
Comment at: clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp:41
+///
jbangert updated this revision to Diff 87579.
jbangert added a comment.
- use iter
https://reviews.llvm.org/D29621
Files:
include/clang/Tooling/RefactoringCallbacks.h
lib/Tooling/RefactoringCallbacks.cpp
unittests/Tooling/RefactoringCallbacksTest.cpp
Index:
jbangert updated this revision to Diff 87580.
jbangert marked 7 inline comments as done.
jbangert added a comment.
Response to initial code review.
https://reviews.llvm.org/D29622
Files:
clang-query/CMakeLists.txt
clang-query/QueryReplace.cpp
clang-query/QueryReplace.h
jbangert added a comment.
Thank you for the feedback, addressed and reformatted using llvm style.
Comment at: clang-query/QueryReplace.h:35-36
+
+ /// \brief Replacement text. %"identifier" will be substituted by the text of
+ /// an identifier.
+ std::string ToTemplate;
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
LGTM. @joerg has raised some legitimate concerns about the Windows
implementation. However that shouldn't hold this cleanup up, since the
implementation is preexisting.
krasimir added inline comments.
Comment at: lib/Format/UnwrappedLineParser.h:121-123
+ // Comments specifies the sequence of comment tokens to analyze. They get
+ // either pushed to the current line or added to the comments before the next
+ // token.
klimek
krasimir added inline comments.
Comment at: lib/Format/UnwrappedLineParser.h:121-123
+ // Comments specifies the sequence of comment tokens to analyze. They get
+ // either pushed to the current line or added to the comments before the next
+ // token.
klimek
malcolm.parsons added inline comments.
Comment at: clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp:20
+namespace {
+class MacroParenthesesPPCallbacks : public PPCallbacks {
+public:
s/MacroParentheses/ConstantValues/ ?
Comment at:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294435: [clang-format] Break before a sequence of line
comments aligned with the next… (authored by krasimir).
Changed prior to commit:
https://reviews.llvm.org/D29626?vs=87615=87622#toc
Repository:
klimek added inline comments.
Comment at: lib/Format/UnwrappedLineParser.h:121-123
+ // Comments specifies the sequence of comment tokens to analyze. They get
+ // either pushed to the current line or added to the comments before the next
+ // token.
krasimir
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294431: [test] Fix hard_link_count test to account for fs
with dir nlink==1 (authored by mgorny).
Changed prior to commit:
https://reviews.llvm.org/D29706?vs=87613=87618#toc
Repository:
rL LLVM
mgorny created this revision.
Filesystems are not required to maintain a hard link count consistent
with number of subdirectories. For example, on btrfs all directories
have nlink==1. Account for that in the test.
Repository:
rL LLVM
https://reviews.llvm.org/D29706
Files:
krasimir updated this revision to Diff 87614.
krasimir added a comment.
- Invent better name for comment analysis.
https://reviews.llvm.org/D29626
Files:
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTest.cpp
Index:
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg.
I'd probably still call it consumeComments or something, as we require a flush
afterwards for the unconsumed comments, but I don't feel too strongly about
that.
Ilod created this revision.
Clang emits a warning when using pure specifier =0 in function definition at
class scope, which is a MS-specific construct, when using -fms-extensions.
However, to detect this, it was using FD->isCanonicalDecl() on function
declaration, which was also detecting
krasimir updated this revision to Diff 87615.
krasimir marked an inline comment as done.
krasimir added a comment.
- Updated call sites.
https://reviews.llvm.org/D29626
Files:
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTest.cpp
Index:
mgorny created this revision.
Herald added subscribers: dberris, aemerson.
Add an #if that excludes the newly added aeabi* tests on non-ARM
platforms. This is consistent with other ARM tests, and aims to make
running builtin tests easier. Lacking a proper infrastructure to run
tests selectively,
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294438: [test] #ifdef new builtin tests for __arm__ platform
(authored by mgorny).
Changed prior to commit:
https://reviews.llvm.org/D29708?vs=87617=87629#toc
Repository:
rL LLVM
aaboud added a comment.
In https://reviews.llvm.org/D16135#669416, @aprantl wrote:
> In https://reviews.llvm.org/D16135#669045, @aaboud wrote:
>
> > Addressed Adrian last comments.
> > Added a LIT tests that covers all the macro kinds:
> >
> > 1. built-in (define)
> > 2. command-line (define,
ABataev added a comment.
The patch is too big and quite hard to review? Could you split it into several
smaller parts?
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4280-4282
+ // We don't need debug information in this function as nothing here refers to
+ // user source
lethalantidote updated this revision to Diff 87484.
lethalantidote marked an inline comment as done.
lethalantidote added a comment.
- Addresses alexfh's comments.
https://reviews.llvm.org/D28973
Files:
clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.cpp
sbarzowski added inline comments.
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:54
+// FIXME use DiagnosticIDs::Level::Note
+diag(NoExceptRange.getBegin(), "in a function declared no-throw here:",
DiagnosticIDs::Note)
+<<
amaiorano added a comment.
In https://reviews.llvm.org/D29221#668867, @klimek wrote:
> +hans
>
> +1 to "format only current document but save all" not making much sense :)
Yes, I've been using this version for a little while now, and it's not useful
to have the current document version. I'll
bmharper updated this revision to Diff 87478.
bmharper added a comment.
small comment tweak
https://reviews.llvm.org/D21279
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D29713
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. I'll commit the patch for you.
https://reviews.llvm.org/D28973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Prazek added a comment.
In https://reviews.llvm.org/D29151#665948, @alexfh wrote:
> In https://reviews.llvm.org/D29151#662504, @zaks.anna wrote:
>
> > Before clang-tidy came into existence the guidelines were very clear. One
> > should write a clang warning if the diagnostic:
> >
> > - can be
dylanmckay marked 2 inline comments as done.
dylanmckay added inline comments.
Comment at: test/CodeGen/avr/target-cpu-defines/atmega328p.c:1
+// RUN: %clang_cc1 -triple avr-unknown-unknown -target-cpu atmega328p
-emit-llvm %s -o - | FileCheck %s
+
asl wrote:
>
dylanmckay updated this revision to Diff 87172.
dylanmckay added a comment.
Clean up the tests
- Dump all defines and search for specific ones
- Remove some unrelated tests. I will commit them separately, as they don't
actually test any behaviour modified in this change.
jlebar added inline comments.
Comment at: lib/Basic/Targets.cpp:1784
+ // atomics. This matches the defaults for the systems using CUDA.
+ // TODO: pass the host target CPU
+ if (HostTriple.getArch() == llvm::Triple::x86)
Someone else is calling
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
This LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D29209
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
aaron.ballman added a comment.
The new tests aren't really what I had in mind. The codegen tests that should
be sema tests can just be rolled into your existing sema tests, by the way.
Comment at: lib/Sema/SemaDeclAttr.cpp:5137
+ if (!checkAttributeNumArgs(S, Attr, 0))
+
dylanmckay updated this revision to Diff 87174.
dylanmckay marked 2 inline comments as done.
dylanmckay added a comment.
Code review
- Remove support for ObjC methods. We shouldn't do this as it doesn't really
make sense
- Move some tests to `Sema`
- Don't attach invalid attributes when it
dylanmckay marked 4 inline comments as done.
dylanmckay added inline comments.
Comment at: test/CodeGen/avr/attributes/interrupt.m:4
+// CHECK: define void @_Z3foov() #0
+void foo() __attribute__((interrupt)) { }
+
aaron.ballman wrote:
> This is not an
mgorny added a comment.
In https://reviews.llvm.org/D28213#666269, @dim wrote:
> > What's the value of `__atomic_always_lock_free(sizeof(long long), 0)` for
> > gcc and clang?
>
> For gcc, it is always 0, for clang (I tested 3.4.1 through 4.0.0) it is
> always 1. Maybe that was always
dim added a comment.
Created https://llvm.org/bugs/show_bug.cgi?id=31864 to continue this on the bug
tracker.
Repository:
rL LLVM
https://reviews.llvm.org/D28213
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
asl requested changes to this revision.
asl added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Driver/MinGWToolChain.cpp:211
+#ifndef LLVM_ON_WIN32
if (GetRuntimeLibType(DriverArgs) == ToolChain::RLT_Libgcc) {
This check
idlecode added inline comments.
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:156
+ safety-no-assembler
+ safety-no-vector-bool
`safety-no-vector-bool` seems to belong to the other patch.
Comment at:
idlecode added inline comments.
Comment at: clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp:63
+cxxOperatorCallExpr(argumentCountIs(1),
+callee(unresolvedLookupExpr()),
+hasArgument(0,
bmharper added a comment.
Thanks @daphnediane. I only read your comment after merging, but that would
have been helpful.
https://reviews.llvm.org/D21279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
mgorny added a comment.
In https://reviews.llvm.org/D29542#666831, @jlebar wrote:
> > Could someone help me figure out what is the cause and correct solution to
> > that failure? @jlebar?
>
> You can see in NVPTXTargetInfo that we read properties from the host
> targetinfo so that we export
asl added inline comments.
Comment at: test/CodeGen/avr/target-cpu-defines/atmega328p.c:1
+// RUN: %clang_cc1 -triple avr-unknown-unknown -target-cpu atmega328p
-emit-llvm %s -o - | FileCheck %s
+
This looks wrong. How you're using FileCheck? Consider dumping
asl added inline comments.
Comment at: test/CodeGen/avr-inline-asm-constraints.c:2
+// REQUIRES: avr-registered-target
+// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s | FileCheck
%s
+
You need checks for multi-character stuff and unsupported
pcc created this revision.
This allows it to be used with the other sanitizers.
https://reviews.llvm.org/D29545
Files:
clang/lib/Driver/Tools.cpp
clang/test/Driver/sanitizer-ld.c
Index: clang/test/Driver/sanitizer-ld.c
===
dylanmckay added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5137
+ if (!checkAttributeNumArgs(S, Attr, 0))
+Attr.setInvalid();
+
aaron.ballman wrote:
> This should simply return rather than attempt to attach an invalid attribute
> to the
jlebar added inline comments.
Comment at: lib/Basic/Targets.cpp:1808
+if (HostTriple.getArch() == llvm::Triple::x86)
+ HostTarget->setCPU("i586");
+
Okay, is this still needed now?
https://reviews.llvm.org/D29542
idlecode added inline comments.
Comment at: lib/Format/Format.cpp:1182
+ I != E; ++I)
+ LastToNextLineFirst[(*I)->Last] = (*std::next(I))->First;
tooling::Replacements Fixes;
Could you add brackets around loop body?
Comment
idlecode requested changes to this revision.
idlecode added a comment.
This revision now requires changes to proceed.
Few other tests (from `ChangeNamespaceTest`) require update too.
https://reviews.llvm.org/D28235
___
cfe-commits mailing list
mgorny created this revision.
Herald added a subscriber: emaste.
Set the maximum width of atomic operations on x86-32 based on the target
CPU. The 64-bit inline atomics require cmpxchg8b which is an i586
instruction. Other inline atomics require cmpxchg which is an i486
instruction.
This fixes
dim added inline comments.
Comment at: lib/Basic/Targets.cpp:4244
+} else // allow locked atomics up to 4 bytes
+ MaxAtomicPromoteWidth = 32;
+ }
Are you purposefully not setting `MaxAtomicInlineWidth` here? (It seems from
`TargetInfo` that the
jlebar added a comment.
> Could someone help me figure out what is the cause and correct solution to
> that failure? @jlebar?
The test is checking that the macros have the same value when compiling for
CUDA host and device. That is, if we're compiling for an x86 CPU and an NVPTX
GPU, we
djasper added inline comments.
Comment at: lib/Format/WhitespaceManager.cpp:190
+struct TokenTypeAndLevel {
+ TokenType Type;
I don't think you need this struct now. Just use the FormatToken itself, it
should have all of this information.
dylanmckay updated this revision to Diff 87191.
dylanmckay removed a reviewer: asl.
dylanmckay added a comment.
Add tests
- Add multichar constraint tests
- Add 'unsupported constraint' tests
https://reviews.llvm.org/D28344
Files:
lib/Basic/Targets.cpp
asl accepted this revision.
asl added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Basic/Targets.cpp:8570
+}
}
don't you want to have "return false" here, just to silence warning for some
compilers?
asl accepted this revision.
asl added a comment.
This revision is now accepted and ready to land.
LGTM with minor style nit
Comment at: lib/Basic/Targets.cpp:8733
+ auto It = std::find_if(AVRMcus.begin(),
+AVRMcus.end(),
+[&](const MCUInfo ) { return
dylanmckay updated this revision to Diff 87190.
dylanmckay marked an inline comment as done.
dylanmckay added a comment.
Add tests
- Add multichar constraint tests
- Add 'unsupported constraint' tests
https://reviews.llvm.org/D28344
Files:
lib/Basic/Targets.cpp
mgorny added inline comments.
Comment at: lib/Basic/Targets.cpp:1808
+if (HostTriple.getArch() == llvm::Triple::x86)
+ HostTarget->setCPU("i586");
+
jlebar wrote:
> Okay, is this still needed now?
Yes. I've specifically tested with it commented out, and
alexfh added a comment.
I wonder whether there's a compiler diagnostic for this purpose. Compiler
diagnostics are more efficient at reaching users and should be preferred where
they are appropriate (this seems like one of such cases).
https://reviews.llvm.org/D29267
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294176: [AVR] Add support for the full set of inline asm
constraints (authored by dylanmckay).
Changed prior to commit:
https://reviews.llvm.org/D28344?vs=87191=87194#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
dylanmckay marked an inline comment as done.
Closed by commit rL294177: [AVR] Allow specifying the CPU on the command line
(authored by dylanmckay).
Changed prior to commit:
bmharper updated this revision to Diff 87179.
bmharper added a comment.
Fixed a stale comment
https://reviews.llvm.org/D21279
Files:
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
compnerd closed this revision.
compnerd added a comment.
SVN r294171
https://reviews.llvm.org/D28407
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
bmharper updated this revision to Diff 87178.
bmharper added a comment.
Hi @djasper,
I've made the latest bunch of review changes, and rebased onto master. I didn't
check the numbers, but I think the code is slightly smaller after the rebase.
Regards,
Ben
https://reviews.llvm.org/D21279
Adi added inline comments.
Comment at: clangd/JSONRPCDispatcher.h:25-32
+ virtual ~Handler() = default;
+
+ /// Called when the server receives a method call. This is supposed to return
+ /// a result on Outs.
+ virtual void handleMethod(llvm::yaml::MappingNode *Params,
klimek added inline comments.
Comment at: clangd/JSONRPCDispatcher.h:25-32
+ virtual ~Handler() = default;
+
+ /// Called when the server receives a method call. This is supposed to return
+ /// a result on Outs.
+ virtual void handleMethod(llvm::yaml::MappingNode *Params,
yaron.keren added a comment.
This code is actually used with Windows as well as Linux (with the exception of
line 218), see the comment blocks above for detailed include dirs from all
platforms from which it was derived.
Please make sure include dirs between gcc and clang match after the
smeenai added a comment.
Ping.
https://reviews.llvm.org/D25208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
smeenai added a comment.
Ping.
https://reviews.llvm.org/D29157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Adi added a comment.
I had some spare time ... I hope you don't mind the comments. Hopefully you
will find something useful :)
Comment at: clangd/ClangDMain.cpp:65-70
+std::vector JSON;
+JSON.resize(Len);
+std::cin.read(JSON.data(), Len);
+
+// Insert a
klimek accepted this revision.
klimek added a comment.
LG. Couple of questions.
Comment at: clangd/ClangDMain.cpp:65
+// Now read the JSON.
+std::vector JSON;
+JSON.resize(Len);
Adi wrote:
> Avoid unnecessary JSON.resize(Len) & potential
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
A couple of nits. Please address Aaron's comment as well.
Comment at: clang-tidy/misc/ThrowWithNoexceptCheck.cpp:53
+ for (const auto : NoExceptRanges) {
+//
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293997: [clang-format] Re-align broken comment lines where
appropriate. (authored by krasimir).
Changed prior to commit:
https://reviews.llvm.org/D29486?vs=86942=86945#toc
Repository:
rL LLVM
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Thanks and sorry if I caused this :(
https://reviews.llvm.org/D29486
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
djasper accepted this revision.
djasper added a comment.
Just a few nits. I think this looks like a great starting point!
Comment at: clangd/ClangDMain.cpp:33
+ llvm::make_unique(Outs, Logs, Store));
+ // FIXME: textDocument/didClose
+ Dispatcher.registerHandler(
krasimir added a comment.
It's very unlikely that you caused this. After re-flowing lots of cases started
going over alternative code paths, and this looks like an instance of that.
https://reviews.llvm.org/D29486
___
cfe-commits mailing list
mati865 added a comment.
Adding
// c:\mingw32\lib\gcc\i686-w64-mingw32\4.9.1\include
// c:\mingw32\lib\gcc\i686-w64-mingw32\4.9.1\include-fixed
for Clang is wrong on Windows because including limits.h will not show an error
and won't really include mingw-w64 limits.h
Output of code from my
filcab added a comment.
Why the switch to `if` instead of a fully-covered switch/case?
In https://reviews.llvm.org/D29369#664426, @vsk wrote:
> In https://reviews.llvm.org/D29369#664366, @regehr wrote:
>
> > Out of curiosity, how many of these superfluous checks are not subsequently
> >
Anastasia added a comment.
@arsenm, do you think you could complete the review?
https://reviews.llvm.org/D29038
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaron.keren added a comment.
Hiding these two include dirs removes many headers. Most has clang equivalents
but not all of them.
For example quadmath.h is only there, and without the include path programs
using it will fail to compile.
The reason mingw limits.h isn't used in your example is
mati865 added a comment.
I know why mingw-w64 limits.h weren't used (check first comment).
At first I was using this hack:
diff -urN clang.orig/3.9.0/include/limits.h clang/3.9.0/include/limits.h
--- clang.orig/3.9.0/include/limits.h 2016-09-26 22:29:13.496441000 +0200
+++
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
If you're still considering to submit this patch, could you rebase it (or maybe
re-generate instead?) and split into easier to digest parts?
A couple of things I noticed:
1.
rsmith added inline comments.
Comment at: include/clang/CodeGen/ModuleBuilder.h:71
+ ///
+ /// This methods can be called before initializing the CGDebugInfo calss.
+ /// But the returned value should not be used until after initialization.
Typo 'calss'
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293800: Drop 'dllimport' when redeclaring inline function
template without the… (authored by hans).
Changed prior to commit:
https://reviews.llvm.org/D29152?vs=85818=86676#toc
Repository:
rL LLVM
hubert.reinterpretcast added a comment.
Ping!
https://reviews.llvm.org/D25674
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
aaboud marked 7 inline comments as done.
aaboud added a comment.
Thanks Adrian and Richard for the comments, I appreciate that.
I am uploading a new patch that address most of the comments.
Adrian, I answered some of your comments below.
Thanks,
Amjad
Comment at:
efriedma added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:72
+ if (const auto *UO = dyn_cast(Op.E))
+return IsPromotedInteger(UO->getSubExpr());
+
Checking isPromotableIntegerType doesn't work the way you want it to; types can
be "promoted"
rnk accepted this revision.
rnk added a comment.
Sorry, missed this, looks good.
Repository:
rL LLVM
https://reviews.llvm.org/D28989
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
aprantl added a comment.
Thanks! Couple more inline comments.
Comment at: lib/CodeGen/CGDebugInfo.h:415
+ /// Get macro debug info.
+ llvm::DIMacro *CreateMacro(llvm::DIMacroFile *Parent, unsigned MType,
It would be good to be a bit more descriptive here.
enyquist added a comment.
Thanks :) I should get a chance to return to this next week.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
bmharper added a comment.
Thanks - the merge conflicts don't look too bad. I should have it cleaned up by
tomorrow.
https://reviews.llvm.org/D21279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
sanwou01 created this revision.
Herald added a subscriber: mehdi_amini.
Add a test for improved inline assembly diagnostics in llvm.
https://reviews.llvm.org/D29415
Files:
test/Misc/inline-asm-diags.c
Index: test/Misc/inline-asm-diags.c
aaboud updated this revision to Diff 86669.
aaboud marked an inline comment as done.
aaboud added a comment.
Address Adrian and Richard comments.
Please, review the new patch and let me know if I need to change anything else.
Thanks,
Amjad
https://reviews.llvm.org/D16135
Files:
801 - 900 of 262385 matches
Mail list logo
