rsmith added inline comments.
Comment at: clang/lib/AST/ExprConstant.cpp:5498-5500
Result.set((Expr*)nullptr, 0, false, true, Offset);
+Result.getLValueDesignator() =
+SubobjectDesignator(E->getType()->getPointeeType());
This is the only caller o
rsmith added a comment.
Do we need to conditionalize this part of libc++? Nothing in the
header appears to need compiler support.
https://reviews.llvm.org/D33538
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin
rsmith added a comment.
In https://reviews.llvm.org/D33538#765045, @rsmith wrote:
> Do we need to conditionalize this part of libc++? Nothing in the
> header appears to need compiler support.
Oh wait, I see what's going on. You're not testing for whether coroutines is
enabled, you're testing
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D33538#765062, @rsmith wrote:
> In https://reviews.llvm.org/D33538#765045, @rsmith wrote:
>
> > Do we need to conditionalize this part of libc++? Nothing in the
> >
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Thanks!
https://reviews.llvm.org/D33568
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
rsmith added a comment.
In https://reviews.llvm.org/D33538#765146, @EricWF wrote:
> In https://reviews.llvm.org/D33538#765045, @rsmith wrote:
>
> > Do we need to conditionalize this part of libc++? Nothing in the
> > header appears to need compiler support.
>
>
> That's correct. I was mistaken
rsmith added a comment.
In https://reviews.llvm.org/D29877#765968, @EricWF wrote:
> I think this patch still gets the following case wrong:
>
> // foo.h
> constexpr struct {
> template void operator()(T) {} // emits unused template warning
> } foo;
>
What specifically do you think is
rsmith added a comment.
In https://reviews.llvm.org/D29877#766196, @EricWF wrote:
> No. But I can point you to `range-v3` which uses this pattern and I think the
> idiom is somewhat appealing, but that's orthogonal to Clang diagnosing it.
I found this:
https://github.com/ericniebler/range-v3/
rsmith added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8977-8980
+ "the return type of 'await_suspend' is required to be 'void' or 'bool' (have
%0)"
+>;
+def note_await_ready_no_bool_conversion : Note<
+ "the return type of 'await_ready' is requir
rsmith added a comment.
Other options:
Do we need to perfectly preserve the functionality of `#pragma once` /
`#import`? That is, would it be acceptable to you if we spuriously enter files
that have been imported in that way, while building a module? If we view them
as pure optimization, we ca
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Serialization/ASTReader.cpp:5870-5876
+TL.setWrittenTypeSpec(
+static_cast(Reader.getRecordData()[Idx++]));
+TL.setWrittenSignSpec(
+
rsmith added a comment.
I like moving `Idx` into the reader, but I have mixed feelings about the
iterator-like interface. For consistency with the rest of the `ASTRecordReader`
interface, and with how we model the writer side, I think perhaps
`Record.ReadInt()` would fit better than using `*Rec
rsmith added a comment.
Looks good, other than error recovery.
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4851
+ DeclarationNameInfo NameInfo(Name, D->getLocation());
+ Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+ DeclContext::lo
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, any chance I can tempt you to lowerCamelCase all the other
ASTRecordReader members to match the new ones as a follow-up change?
https://reviews.llvm.org/D27836
__
rsmith added inline comments.
Comment at: include/clang/AST/Decl.h:1923
+IsConstexpr = IC;
+setImplicitlyInline();
+ }
nwilson wrote:
> hubert.reinterpretcast wrote:
> > I am quite sure this is not the right thing to do when `IC` is `false`.
> Good point
rsmith added a comment.
Why are you adding a language option for this? Just use `!LangOpts.CPlusPlus`.
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added inline comments.
Comment at: lib/Sema/SemaInit.cpp:884
+bool MissingBracesOkay = false;
+
+if (!SemaRef.getLangOpts().CPlusPlus &&
Remove empty line.
Comment at: lib/Sema/SemaInit.cpp:885-892
+if (!SemaRef.getLangOpts()
rsmith added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:7464-7470
+const Type *NonAttributedFTy = R.getTypePtr();
+while (const auto *AttrTy = NonAttributedFTy->getAs()) {
+ NonAttributedFTy = AttrTy->getModifiedType().getTypePtr();
+}
bool HasProtot
rsmith added a comment.
Looks good to go once Daniel's and my comments are addressed. Thank you.
Comment at: lib/AST/Expr.cpp:1887
+bool InitListExpr::isIdiomaticZeroInitializer(const LangOptions &LangOpts)
const {
+ assert(!getSyntacticForm() && "only test syntactic form as
rsmith added inline comments.
Comment at: lib/Basic/Diagnostic.cpp:179
+
+ // 2nd most frequent case: L is before the first diag state change.
+ FullSourceLoc FirstStateChangePos = DiagStatePoints[1].Loc;
It's surprising to me that this would be particularly fr
rsmith added a comment.
The test failure in test/CodeGen/microsoft-call-conv-x64.c definitely indicates
a problem. The code has defined behavior, but the IR you say we now produce has
undefined behavior due to a type mismatch between the call and the callee.
It looks to me like unprototyped `__
rsmith added a comment.
In https://reviews.llvm.org/D28166#633621, @aaron.ballman wrote:
> Do you think this patch should be gated on (or perhaps combined with) a fix
> for the lowering bug, or do you think this patch is reasonable on its own?
> Given that it turns working code into UB, I think
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, but a minor simplification seems possible.
Comment at: include/__tuple:32
template
-class _LIBCPP_TYPE_VIS_ONLY tuple_size
-: public __tuple_size_base_type<_Tp>::
rsmith added a comment.
In https://reviews.llvm.org/D28166#634196, @aaron.ballman wrote:
> So I think the correct behavior is to only enable the vararg behavior when
> the function is variadic with an ellipsis rather than variadic due to a lack
> of prototype.
That sounds right. Note that fun
rsmith added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:3547
NoexceptRange = SourceRange(KeywordLoc, T.getCloseLocation());
-} else {
- NoexceptType = EST_None;
}
} else {
Should `NoexceptRange` be set in the `else` case too,
rsmith added inline comments.
Comment at: lib/Basic/Diagnostic.cpp:179
+
+ // 2nd most frequent case: L is before the first diag state change.
+ FullSourceLoc FirstStateChangePos = DiagStatePoints[1].Loc;
djasper wrote:
> rsmith wrote:
> > It's surprising to me
rsmith added a comment.
Sorry I missed this =( Thanks for the fix!
https://reviews.llvm.org/D26893
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added a comment.
Looks OK. Is it possible to add a test case for this without
https://reviews.llvm.org/D20428? If not, this is small enough that rolling it
into https://reviews.llvm.org/D20428 (so it can be committed with its testcase)
would make sense.
https://reviews.llvm.org/D28258
rsmith added a comment.
Can you add a test please?
https://reviews.llvm.org/D28427
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added a comment.
This makes sense to me in principle, but I'd like @chandlerc to weigh in.
https://reviews.llvm.org/D26244
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added inline comments.
Comment at: include/__string:248
+#if _LIBCPP_STD_VER <= 14
+return (const char_type*) memchr(__s, to_int_type(__a), __n);
+#else
We can add another builtin to Clang to support this case if you'd like.
(There's also a way to ge
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D28427
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Lex/HeaderSearch.cpp:1082
+ auto TryEnterImported = [&](void) -> bool {
+if (!ModulesEnabled)
Maybe add a FIXME here indicating th
rsmith added inline comments.
Comment at: lib/Sema/SemaTemplate.cpp:1214-1215
CXXRecordDecl::Create(Context, Kind, SemanticContext, KWLoc, NameLoc, Name,
PrevClassTemplate?
PrevClassTemplate->getTemplatedDecl() : nullpt
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Yes, this is correct; per [stmt.dcl]/5, the destructor only runs if the object
was constructed.
Repository:
rL LLVM
https://reviews.llvm.org/D28505
_
rsmith added inline comments.
Comment at: include/clang/Parse/Parser.h:1432
+ ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast,
+ bool IsLambdaExprForbidden = false);
ExprResult ParseConstraintExpression();
rsmith added a comment.
Thanks! I assume there's still no measurable performance impact?
Comment at: include/clang/AST/ODRHash.h:54
+ // in Pointers.
+ size_type NextFreeIndex;
+
Is this always the same as `Pointers.size()`?
Comment at: lib
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:301
bit DuplicatesAllowedWhileMerging = 0;
+ // Set to true if this attribute should apply to template declarations,
+ // remains false if this should only be applied to the definition.
I
rsmith added a comment.
This needs a test case, but the change itself looks fine to me.
Repository:
rL LLVM
https://reviews.llvm.org/D31069
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, do you need someone to commit for you?
https://reviews.llvm.org/D31069
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM
https://reviews.llvm.org/D30848
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
> - The patch-as-is checks whether pragmas should be demoted to warnings for
> all AST files, not just implicit modules. I can add a bit of logic to
> `ReadPragmaDiagnosticMappings` that
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298477: Suppress warning on unreachable
[[clang::fallthrough]] within a template… (authored by rsmith).
Changed prior to commit:
https://reviews.llvm.org/D31069?vs=92104&id=92588#toc
Repository:
rL L
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
The patch itself LGTM. I'd like some test coverage, but if this will be covered
by some upcoming interpreter piece and you need this to unblock yourselves,
that's good enough for me. In any ca
rsmith added inline comments.
Comment at: lib/Sema/Sema.cpp:473
if (const FunctionDecl *FD = dyn_cast(D)) {
+// If this is a function template and neither of its specs is used, warn.
+if (FunctionTemplateDecl *Template = FD->getDescribedFunctionTemplate())
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM with the overloaded operator check removed.
https://reviews.llvm.org/D29877
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Can you also add a basic test that this works in C? Thanks!
https://reviews.llvm.org/D31781
___
cfe-commits mailing list
cfe-commits@lists.llvm.o
rsmith accepted this revision.
rsmith added a comment.
LGTM with one change.
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:95
def err_drv_force_crash : Error<
- "failing because environment variable '%0' is set">;
+ "failing because %select{environment variable|op
rsmith added inline comments.
Comment at: lib/Serialization/ASTReader.cpp:8487
+std::sort(Comments.begin(), Comments.end(),
+ BeforeThanCompare(SourceMgr));
Context.Comments.addDeserializedComments(Comments);
Does this cause us to deserializ
This revision was automatically updated to reflect the committed changes.
Closed by commit rL300264: Diagnose attempt to take address of bitfield members
in anonymous structs. (authored by rsmith).
Changed prior to commit:
https://reviews.llvm.org/D27263?vs=79759&id=95222#toc
Repository:
rL
rsmith added a comment.
The change to test/SemaCXX/anonymous-struct.cpp appeared to be unrelated to the
rest of the patch, so I committed it separately as r300266.
Thank you!
Repository:
rL LLVM
https://reviews.llvm.org/D27263
___
cfe-commits m
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:869
def GNUInline : InheritableAttr {
- let Spellings = [GCC<"gnu_inline">];
+ let Spellings = [GCC<"gnu_inline">, Declspec<"inline">];
let Subjects = SubjectList<[Function]>;
aaron.ballm
rsmith added a comment.
In https://reviews.llvm.org/D32092#727543, @zahiraam wrote:
> Pushed the submit too fast ...
> Before I submitted this review, I have done some experiments and inline and
> declspec(inline) have the same behavior. Compiling with /Ob0 disables
> inlining. With -O1 or -O2
rsmith added a comment.
From some very superficial testing, it looks like CL treats
`__declspec(inline)` exactly like a synonym for `inline` (it even rejects them
both appearing on the same declaration, complaining about a duplicate `inline`
specifier). So modeling this as `GNUInlineAttr` is de
rsmith added inline comments.
Comment at: include/clang/AST/ExprCXX.h:4237
+ /// compiler.
+ bool IsImplicitlyCreated : 1;
+
I would go with just `isImplicit`, to match other similar uses throughout the
AST. Also maybe sink this into the `Stmt` bitfields to ma
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Thanks, looks good assuming your performance testing doesn't uncover anything.
Comment at: lib/AST/ODRHash.cpp:319-321
+if (!D) return;
+if (D->isImplicit())
+ r
rsmith added a comment.
Does OS X have the C11 `aligned_alloc` function? Perhaps we could use that
instead, when available.
https://reviews.llvm.org/D28931
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
rsmith added inline comments.
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:229
+def remark_ssp_applied_reason
+: Remark<"SSP applied to function due to %select{an unknown reason|a "
+ "call to alloca|a stack allocated buffer or struct containing a "
rsmith added inline comments.
Comment at: include/clang/Sema/Overload.h:758
/// instead.
+/// FIXME: Now that it only alloates ImplicitConversionSequences, do we
want
+/// to un-generalize this?
Typo "alloates"
Comment at: lib/Sem
rsmith added a comment.
Generally looks good, but we have a better way of modeling types with a
trailing variable-length array that you should use.
Comment at: include/clang/AST/StmtCXX.h:299
/// down the coroutine frame.
class CoroutineBodyStmt : public Stmt {
enum SubSt
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.
I don't think it's possible to check this in the way you're doing so here. In
general, there's no way to know whether a constant expression will be part of a
`typedef` declaration or
rsmith added a comment.
Can we instead address this locally in `_Pragma` handling, by getting it to
clear out the junk it inserted into the token stream when it's done (if
backtracking is enabled)?
Repository:
rL LLVM
https://reviews.llvm.org/D28772
__
rsmith added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8708
+def note_redefinition_modules_same_file : Note<
+ "'%0' included multiple times, additional (likely non-modular) include
site in module '%1'">;
+def note_redefinition_modules_same_fi
rsmith added a comment.
In https://reviews.llvm.org/D22587#551647, @mgehre wrote:
> I'm sorry if this sounds dumb, but is there a way for me to follow that
> particular discussion?
Only if you have access to the C++ committee's internal reflectors. Sadly this
is not on the issues list yet eit
rsmith added a comment.
In https://reviews.llvm.org/D21675#654659, @teemperor wrote:
> Would be nice if we could make Stmt::Profile, ODRHash and the CloneDetection
> use the same backend.
This code is *already* reusing the Stmt::Profile code for hashing of
statements. Why was a different mech
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
As of r293254, the `-G=` and `-msmall-data-threshold=` flags are just aliases
of `-G`, so you don't need those parts of this patch any more. The PPC part
looks fine, but please add a testcase.
rsmith added a comment.
LGTM
https://reviews.llvm.org/D26345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added a comment.
Another "fun" testcase:
struct S {
void operator++(int n) _diagnose_if(n, "wat", "warning");
};
void f(S s) {
s++; // no warning
s.operator++(1); // warning
}
Comment at: include/clang/Sema/Sema.h:2638
- /// Check the diagnose_if
rsmith added a comment.
Comment at: include/clang/AST/ASTContext.h:2490
/// it is not used.
- bool DeclMustBeEmitted(const Decl *D);
+ bool DeclMustBeEmitted(const Decl *D, bool WritingModule = false);
I think the name of this flag might be out of sync
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/AST/ExternalASTSource.cpp:33
+ExternalASTSource::hasExternalDefinitions(unsigned ID) {
+ return EK_ReplyHazy;
+}
dblaikie wrote:
> rsmit
rsmith added inline comments.
Comment at: clang/test/SemaCXX/pr30306.cpp:5
+template
+void g(T) { int a[f(3)]; } // expected-no-diagnostics
+
Shouldn't we be (somehow) handling the cleanups from the array bound expression
here -- either discarding them or attac
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'
rsmith added inline comments.
Comment at: include/clang/Lex/Preprocessor.h:310
+
+const Stack &getStack() const {
+ assert(ConditionalStack);
Return an `ArrayRef` rather than exposing the type of the storage (which is an
implementation detail), here and
rsmith added inline comments.
Comment at: lib/Parse/ParseDecl.cpp:2702
DS.SetRangeStart(Tok.getLocation());
-DS.SetRangeEnd(SourceLocation());
+DS.SetRangeEnd(Tok.getLocation());
}
This doesn't look right: this is a source range containing exactly
rsmith added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:373-391
+class TemplateDeclWithACBase {
+protected:
+ TemplateDeclWithACBase() = default;
+
+ ConstrainedTemplateDeclInfo CTDInfo;
+};
+
This mechanism seems unnecessary to me; allocatin
rsmith added inline comments.
Comment at: lib/AST/ExprConstant.cpp:5061
+ APValue RVal;
+ // FIXME: We need to make sure we're passing the right type that
+ // maintains cv-qualifiers.
faisalv wrote:
> I don't think we need this fixme -
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Sema/SemaTemplate.cpp:1300
+ // Attach the associated constraints when the declaration will not be part of
+ // a decl chain
+ Expr *const ACtoAttach =
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D22057#543199, @Sunil_Srivastava wrote:
> Now: Why the InstantiationIsPending bit is not precisely tracking the
> presence in the PendingInstantiations list?
[...
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:301
bit DuplicatesAllowedWhileMerging = 0;
+ // Set to true if this attribute should apply to template declarations,
+ // remains false if this should only be applied to the definition.
er
rsmith added inline comments.
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:233
"AddressSanitizer doesn't support linking with debug runtime libraries yet">;
+def note_drv_supported_values : Note<"supported values are:">;
+def note_drv_supported_value_with_descripti
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, I think this is also OK for Clang 4 if Hans is willing to take it.
Comment at: lib/AST/ExprConstant.cpp:607-612
+ /// Evaluate as a constant expression. In certain
rsmith added a comment.
Looks good, though there are some `value_type` constructors left. I've not
checked the standard to see if they are all declared with `charT`.
Comment at: include/string:782
_LIBCPP_INLINE_VISIBILITY
basic_string(const value_type* __s, size_typ
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Frontend/CompilerInvocation.cpp:1753-1754
+ KindValue != LangStandard::lang_unspecified;
+ ++KindValue)
+ {
+const LangStan
rsmith added a comment.
LGTM, do you need someone to commit for you?
https://reviews.llvm.org/D29724
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added inline comments.
Comment at: include/string:782
_LIBCPP_INLINE_VISIBILITY
basic_string(const value_type* __s, size_type __n);
_LIBCPP_INLINE_VISIBILITY
EricWF wrote:
> rsmith wrote:
> > Did you skip this one intentionally?
> Yes. `size
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/AST/ExprConstant.cpp:428-429
+llvm::DenseMap LambdaCaptureFields;
+FieldDecl *LambdaThisCaptureField;
+
I'm a little concerned
rsmith added a comment.
I don't like this name; it sounds too much like you're asking whether a certain
direct-initialization is possible, which is what `__is_constructible` does. I
also don't like the idea of combining an "is this type direct-initializable
from this list of arguments" check wi
rsmith added a comment.
Committed as r295113.
https://reviews.llvm.org/D29724
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith added a comment.
In https://reviews.llvm.org/D28510#654821, @faisalv wrote:
> In https://reviews.llvm.org/D28510#653794, @rsmith wrote:
>
> > I don't think it's possible to check this in the way you're doing so here.
> > In general, there's no way to know whether a constant expression wil
rsmith added a comment.
It seems to me that the problem here is that `DeclMustBeEmitted` is not safe to
call in the middle of deserialization if anything partially-deserialized might
be reachable from the declaration we're querying, and yet we're currently
calling it in that case. I don't see h
rsmith added a comment.
Other than (5), all the failing cases look like they should fail per the
current `basic_string` spec.
Comment at:
test/std/strings/basic.string/string.cons/implicit_deduction_guides.pass.cpp:57
+ { // Testing (2)
+// FIXME: (2) doesn't work with i
rsmith added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:2478
break;
+case Type::DeducedTemplateSpecialization: {
+ QualType DT = cast(T)->getDeducedType();
EricWF wrote:
> I'll put this in alphabetical order before committing.
Reuse
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
https://reviews.llvm.org/D30082
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:258
+ /// A block containing unhashed contents. It currently holds Diagnostic
+ /// Options and Sign
This revision was automatically updated to reflect the committed changes.
Closed by commit rL295794: Fix assertion failure when generating debug
information for a variable (authored by rsmith).
Changed prior to commit:
https://reviews.llvm.org/D30082?vs=88943&id=89300#toc
Repository:
rL LLVM
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
It might make sense to move the *Arch.cpp files to a subdirectory of
lib/Driver, but otherwise this looks good.
https://reviews.llvm.org/D30315
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D29812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D20710
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM with a couple of changes.
Comment at: test/Modules/Inputs/merge-using-decls/a.h:25
+#if __cplusplus <= 199711L // C++11 does not allow access declerations
template st
rsmith accepted this revision.
rsmith added a comment.
Yes, I'm OK with this as a stopgap fix for 4.0. I would have preferred a more
full fix for 4.0 but we've run out of time for that, and the PCH case does seem
more pressing.
https://reviews.llvm.org/D29753
___
1 - 100 of 188 matches
Mail list logo
