rjmccall accepted this revision.
rjmccall added a comment.
LGTM, thanks!
Repository:
rC Clang
https://reviews.llvm.org/D47166
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall added a comment.
I thought we already had places in Sema that marked inline virtual methods as
used, instantiated templates, etc. for devirtualization purposes when
optimization was enabled. Did we rip that out?
The problem we've had over and over with devirtualization is that we have
rjmccall added a reviewer: akyrtzi.
rjmccall added a comment.
CC: Argyrios for the USR question.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
rjmccall added a comment.
The changes to Clang generally seem reasonable, but I think you should split
them into a separate commit from the commit that adds the intrinsic itself.
Comment at: clang/lib/CodeGen/CGExpr.cpp:3858
+}
+ }
+
Please add a comment
rjmccall added a comment.
There are at least three good reasons to make sure this can enabled/disabled by
a flag:
- We have to anticipate that introducing new keywords will cause some
compatibility problems. New language standards that introduce new keywords can
be disabled using `-std=`. We
rjmccall added a comment.
In https://reviews.llvm.org/D47108#1109145, @Prazek wrote:
> In https://reviews.llvm.org/D47108#1109014, @rjmccall wrote:
>
> > I thought we already had places in Sema that marked inline virtual methods
> > as used, instantiated templates, etc. for devirtualization purp
rjmccall added inline comments.
Comment at: clang/lib/CodeGen/CGExpr.cpp:3858
+}
+ }
+
Prazek wrote:
> rjmccall wrote:
> > Please add a comment explaining why this is necessary. (I'm actually not
> > sure why it is, because surely the invariant groups we g
rjmccall added inline comments.
Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+ "Fixed point types are only allowed in C">;
leonardchan wrote:
> leonardchan wrote:
> > leonardchan wrote:
> > > rsmi
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rC Clang
https://reviews.llvm.org/D47354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38456
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM, but is there a reason this isn't just part of that patch?
Repository:
rL LLVM
https://reviews.llvm.org/D38460
___
cfe-commits mailin
rjmccall added a comment.
I assume I should wait on reviewing this until all of these smaller TBAA
patches land?
https://reviews.llvm.org/D37826
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall added inline comments.
Comment at: include/clang/CodeGen/CodeGenABITypes.h:81
+// Returns a field number for a struct suitable for GEP'ing
+unsigned getFieldNumber(CodeGenModule &CGM,
"Given a non-bitfield struct field, return its index within the elem
rjmccall added a comment.
Why is most of this patch necessary under the design of adding a non-canonical
__private address space?
Comment at: include/clang/AST/Type.h:336
+ /// space makes difference.
+ bool getImplicitAddressSpaceFlag() const { return Mask & IMask; }
+ voi
rjmccall added a comment.
In https://reviews.llvm.org/D38473#888597, @rsmith wrote:
> In https://reviews.llvm.org/D38473#888159, @mppf wrote:
>
> > > You should also indicate *which* record layout (complete object type or
> > > base subobject type) the field number is for. I don't think there's
rjmccall added a reviewer: dexonsmith.
rjmccall added a comment.
I'll let Duncan answer that question.
https://reviews.llvm.org/D31363
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall added a comment.
Are you sure it's a good idea to not print the address space when it's
implicit? Won't that often lead to really confusing diagnostics?
Also, we do already have a way of expressing that an extended qualifier was
explicit: AttributedType. We have very similar sorts of
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM with one comment fix that I noticed.
Comment at: lib/CodeGen/CodeGenModule.h:659
- llvm::MDNode *getTBAAInfoForVTablePtr();
+ /// getTBAAAccessInfo - Get TBAA in
rjmccall added a comment.
In https://reviews.llvm.org/D37826#888086, @kosarev wrote:
> In https://reviews.llvm.org/D37826#887820, @rjmccall wrote:
>
> > I assume I should wait on reviewing this until all of these smaller TBAA
> > patches land?
>
>
> These small patches are actually part of this
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D37826
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall added a comment.
I don't see any reason not to use linkonce_odr + hidden even outside of -Oz.
Otherwise LGTM.
https://reviews.llvm.org/D38606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
rjmccall added a comment.
You have an important backend change relying on being able to preserve type
sugar better in diagnostics? The only apparent semantic change in this patch
is that you're changing the mangling, which frankly seems incorrect.
https://reviews.llvm.org/D35082
__
rjmccall added a comment.
Non-canonical information is not supposed to be mangled.
It's not clear to me what the language rule you're really trying to implement
is. Maybe you really do need a canonical __private address space, in which
case you are going to have to change a lot of code in Sema
rjmccall added a comment.
Okay. I think I see your point about why it would be better to have a
canonical __private address space that is different from the implicit address
space 0. I assume this means that there should basically never be pointers,
references, or l-values in address space 0
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks. LGTM.
https://reviews.llvm.org/D38606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
rjmccall added a comment.
In https://reviews.llvm.org/D35082#890359, @Anastasia wrote:
> In https://reviews.llvm.org/D35082#890162, @rjmccall wrote:
>
> > Okay. I think I see your point about why it would be better to have a
> > canonical __private address space that is different from the impli
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D38659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks, this is a nice improvement.
Repository:
rL LLVM
https://reviews.llvm.org/D38695
___
cfe-commits mailing list
cfe-commits@lists.llv
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D38473
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Sure.
https://reviews.llvm.org/D38733
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall added a comment.
It sounds like there's agreement about the basic technical direction of
introducing LangAS::opencl_private. Please introduce isAddressSpaceImplicit()
in a different patch and make this patch just about the introduction of
LangAS::opencl_private. You can have the pret
rjmccall added a comment.
In https://reviews.llvm.org/D35082#895312, @yaxunl wrote:
> Thanks. I will separate the implicit addr space flag to another patch.
Thanks, appreciated.
Comment at: include/clang/AST/Type.h:562
+ static const uint32_t IMask = 0x200;
+ static const
rjmccall added a comment.
A few minor comments; feel free to commit after addressing them.
Comment at: include/clang/Basic/AddressSpaces.h:34
// OpenCL specific address spaces.
opencl_global,
yaxunl wrote:
> rjmccall wrote:
> > I think you need a real c
rjmccall added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:2263
+ }
+}
+
Is there a reason this only fails on x86? If LLVM doesn't have generic
wide-operation lowering, this probably needs to be a target whitelist rather
than a blacklist.
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38788
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38791
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38793
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38795
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Yes, thank for you handling the cast case well.
Repository:
rL LLVM
https://reviews.llvm.org/D38796
___
cfe-commits mailing list
cfe-commi
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
This function feels increasingly poorly-named, but let's leave that alone for
now.
Repository:
rL LLVM
https://reviews.llvm.org/D38794
__
rjmccall added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5226
+def err_objc_variable_sized_type_not_at_end : Error<
+ "field %0 with variable sized type %1 is not at the end of class">;
+def note_next_field_declaration : Note<
rjmccall added a comment.
Okay. Sounds good to me.
We intend to actually implement the generic lowering, I hope?
https://reviews.llvm.org/D38861
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
rjmccall added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:15055
}
+ // If it is the last field is checked elsewhere.
}
vsapsai wrote:
> rjmccall wrote:
> > "Whether" rather than "If", please. You should also leave a com
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rL LLVM
https://reviews.llvm.org/D38945
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Ok.
https://reviews.llvm.org/D38796
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D38966
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:3665
-LValue CodeGenFunction::EmitLValueForField(LValue base,
- const FieldDecl *field) {
- LValueBaseInfo BaseInfo = base.getBaseInfo();
- AlignmentSource fieldAlignS
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Okay, thanks.
https://reviews.llvm.org/D38947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
rjmccall added inline comments.
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:5095
+fieldType = fieldType->getAsArrayTypeUnsafe()->getElementType();
+ }
+
You can't just use isa<> here; there can be typedefs of incomplete array type.
https://reviews.llvm.org
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D38773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall added inline comments.
Comment at: include/clang/AST/Type.h:337
+Mask = (Mask & ~ImplicitAddrSpaceMask) |
+ (((uint32_t)Value) << ImplicitAddrSpaceShift);
+ }
This is probably cleaner as:
Mask = (Value ? (Mask | ImplicitAddrSpaceMask) :
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Well, I've been agreeing so far that it makes sense to propagate TBAA
information separately from LValueBaseInfo, and this seems like a logical part
of that, so okay.
Repository:
rL LL
rjmccall added a comment.
Hmm. I'm not sure I like the design of merging TBAAAccessInfo into
LValueBaseInfo. LValueBaseInfo is currently the set of information that's
generally preserved across l-value manipulations. It was extracted from LValue
specifically to create an encapsulated entity
rjmccall added a comment.
If this is something we generally need to be doing in all the places we
temporarily save and restore the insertion point, we should fix the basic
behavior of saveIP instead of adding explicit code to a bunch of separate
places. Can we just override saveIP() on CGBuild
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D39184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rjmccall added a comment.
The code looks fine to me. The test seems very vague to me, but I'd like Dave
to weigh in on that, because I'm not sure it's reasonable to test the exact
pattern here.
Also, Dave, I don't know what the IR invariants around debug info are. Is this
something we should
rjmccall added inline comments.
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:5095
+fieldType = fieldType->getAsArrayTypeUnsafe()->getElementType();
+ }
+
vsapsai wrote:
> rjmccall wrote:
> > You can't just use isa<> here; there can be typedefs of incomplete a
rjmccall added a comment.
In https://reviews.llvm.org/D39069#904507, @probinson wrote:
> Anytime the code between saveIP() and restoreIP() could set the current debug
> location, it needs to be saved/restored along with the insertion point. I
> have to say the problem is not obvious to me here
rjmccall added a comment.
AFAIK, this is pointless because that alloca will be trivially eliminated by
mem2reg. Am I missing something? Is this important for some sort of
consistency check?
Repository:
rL LLVM
https://reviews.llvm.org/D39138
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExpr.cpp:2171
+ LValueBaseInfo *ReferenceeBaseInfo,
+ TBAAAccessInfo *ReferenceeTBAAInfo) {
+ llvm::LoadInst *Load = Builder.CreateLoad(ReferenceAddr
rjmccall added a comment.
Okay, if this is just for your own checking, I'd rather not take it. It's not
a significant compile-time cost, but there's no reason to pay it at all.
Repository:
rL LLVM
https://reviews.llvm.org/D39138
___
cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Okay, LGTM.
https://reviews.llvm.org/D39069
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
rjmccall added a comment.
In https://reviews.llvm.org/D39138#905184, @hfinkel wrote:
> In https://reviews.llvm.org/D39138#904747, @rjmccall wrote:
>
> > Okay, if this is just for your own checking, I'd rather not take it. It's
> > not a significant compile-time cost, but there's no reason to pa
rjmccall added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:4924
+<< Callee->getSourceRange();
+ }
+
Why is the diagnostic at the end location? And why are you separately checking
whether it's ignored at the begin location?
rjmccall added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:10668
+ if (Source->isAtomicType() || Target->isAtomicType())
+S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+
Why would the target be an atomic type? And if it is an atom
rjmccall added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:10668
+ if (Source->isAtomicType() || Target->isAtomicType())
+S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
+
jfb wrote:
> rjmccall wrote:
> > Why would the target be an a
rjmccall added a comment.
It says the type of the assignment expression, not the type of the LHS.
C11 [6.5.16]p2: "The type of an assignment expression is the type the left
operand would have after lvalue conversion."
C11 [6.3.2.1]p2: "...this is called lvalue conversion. If the lvalue has
qua
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM, with one minor suggestion.
Comment at: lib/Sema/SemaExpr.cpp:5745
+// C99 6.5.2.5p6: Function scope compound literals must have automatic
+// storage which
rjmccall added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:5745
+// C99 6.5.2.5p6: Function scope compound literals must have automatic
+// storage which generally excludes address space-qualified ones.
+Diag(LParenLoc, diag::err_compound_literal_with_address_s
rjmccall accepted this revision.
rjmccall added a comment.
LGTM.
https://reviews.llvm.org/D51426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:4924
+<< Callee->getSourceRange();
+ }
+
rjmccall wrote:
> Why is the diagnostic at the end location? And why are you separately
> checking whether it's ignored at the begin location
rjmccall added inline comments.
Comment at: include/clang/AST/Decl.h:977
+
+unsigned EscapingByref : 1;
};
This doesn't actually seem to be initialized anywhere.
Comment at: include/clang/AST/Decl.h:1422
+
+ bool isNonEscapingByref() co
rjmccall added a comment.
In https://reviews.llvm.org/D51200#1223768, @kuhar wrote:
> In https://reviews.llvm.org/D51200#1223752, @rsmith wrote:
>
> > +rjmccall for CodeGen changes
>
>
> @rsmith @rjmccall
> I have one high-level question about the CodeGen part that I wasn't able to
> figure ou
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: include/clang/AST/Decl.h:977
+
+unsigned EscapingByref : 1;
};
ahatanak wrote:
> rjmccall wrote:
> > This doesn't actually seem
rjmccall closed this revision.
rjmccall added a comment.
Committed as r341489.
Repository:
rC Clang
https://reviews.llvm.org/D44539
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall closed this revision.
rjmccall added a comment.
Committed as r341491.
https://reviews.llvm.org/D51426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: lib/Sema/SemaChecking.cpp:10974
+ if (E->IgnoreParenImpCasts()->getType()->isAtomicType())
+return;
CheckImplicitConversion(S, E->IgnoreParenImp
rjmccall added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4111
+ "candidate template ignored: %select{template is not a function template"
+ "|is not a member of the enclosing namespace}0">;
Your first explanation has a subj
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCUDANV.cpp:444
+auto HandleValue =
+CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign());
+llvm::Constant *Zero =
llvm::Constant::getNullValue(HandleValue->getType());
yax
rjmccall added a comment.
Hmm. I think the approach of flagging ICEs that are semantically part of an
explicit cast is probably a better representation for tools across the board.
If we *are* going to do it this way, though, I think you should (1) make the
collection of skipped expressions opt
rjmccall added a comment.
In https://reviews.llvm.org/D49508#1168599, @lebedev.ri wrote:
> In https://reviews.llvm.org/D49508#1168584, @rjmccall wrote:
>
> > Hmm. I think the approach of flagging ICEs that are semantically part of
> > an explicit cast is probably a better representation for too
rjmccall added inline comments.
Comment at: docs/Block-ABI-Apple.rst:64
enum {
+BLOCK_IS_NOESCAPE = (1 << 23),
BLOCK_HAS_COPY_DISPOSE = (1 << 25),
Something happened to my older comments here, but please document the meaning
of this
rjmccall added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:3150
+ !getLangOpts().OpenCLCPlusPlus)
+return false;
+
yaxunl wrote:
> rjmccall wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > It's not really OpenCL C++ that's special here,
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4111
+ "candidate template ignored: %select{template is not a function template"
+ "|is not a member
rjmccall added inline comments.
Comment at: lib/CodeGen/CGCUDANV.cpp:466
+ CtorBuilder.CreateStore(RegisterFatbinCall, GpuBinaryAddr);
+ CtorBuilder.CreateBr(ExitBlock);
+CtorBuilder.SetInsertPoint(ExitBlock);
I meant more putting all the code for I
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks, looks good.
https://reviews.llvm.org/D49294
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks. A couple tiny things and then LGTM.
Comment at: docs/Block-ABI-Apple.rst:69
+// block is a no-op, which is exactly how global blocks are handled.
+
+
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Alright, thanks, LGTM.
https://reviews.llvm.org/D49083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/
rjmccall added inline comments.
Comment at: include/clang/AST/Stmt.h:205
unsigned Kind : 6;
-unsigned BasePathSize : 32 - 6 - NumExprBits;
+bool PartOfExplicitCast : 1;
+unsigned BasePathSize : 32 - 6 - 1 - NumExprBits;
This needs to be `unsigned
rjmccall added inline comments.
Comment at: include/clang/AST/Stmt.h:206
+bool PartOfExplicitCast : 1;
+unsigned BasePathSize : 32 - 6 - 1 - NumExprBits;
};
lebedev.ri wrote:
> rjmccall wrote:
> > This needs to be serialized.
> Uhm, could you please ex
rjmccall added inline comments.
Comment at: lib/CodeGen/CodeGenFunction.h:380
+ /// True if sanitizer checks for current pointer value are generated.
+ bool PointerChecksAreEmitted = false;
+
I don't think this is a good way of doing this. Using global state m
rjmccall added a comment.
The commit message here could be better. You're passing `-target-cpu` when
running the device-mode compiler.
https://reviews.llvm.org/D49643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks. This looks reasonable to me.
https://reviews.llvm.org/D49643
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.ll
rjmccall added a comment.
Sorry, I'm not following you. Are you doing some sort of type-based security
analysis based on LLVM IR types?
https://reviews.llvm.org/D49403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
rjmccall added a comment.
I honestly don't know why the `BlockDecl` is in the members list in the first
place; that seems wrong, for the same reason that we wouldn't (I assume?)
consider a lambda's implicit record to be a member.
Repository:
rC Clang
https://reviews.llvm.org/D49688
_
rjmccall added a comment.
Are you sure you shouldn't do it based on some sort of actual annotation based
on the frontend's knowledge of types instead of adding semantics for LLVM IR
types that were never intended?
https://reviews.llvm.org/D49403
_
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
It makes sense that the BlockDecl's parent DC is the class, although I think it
would be even better if we took a page from Swift and make a special DC for
initializer expressions. I don'
rjmccall added a comment.
In https://reviews.llvm.org/D49718#1173038, @ahatanak wrote:
> Note that in order to destruct C++ objects, I'm using pushDestroy which
> pushes a NormalCleanup when exception handling isn't enabled. This is
> different from PushDestructorCleanup which always pushes a
101 - 200 of 3247 matches
Mail list logo
