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.
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 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/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 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 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 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 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 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.
In https://reviews.llvm.org/D29951#750885, @v.g.vassilev wrote:
> In order to create a reasonable test I need to use
> `-error-on-deserialized-decl` and I hit a bug:
> https://bugs.llvm.org/show_bug.cgi?id=32988
>
> Richard, could you help me out with the test? Maybe we
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
I like it, and it seems like it would nicely generalize if there are more cases
that we find need this treatment.
Comment at: clang/lib/Lex/LiteralSupport.cpp:676-682
-
rsmith added inline comments.
Comment at: lib/Sema/SemaExprCXX.cpp:2661-2669
+// If we're in local visibility mode, we reuse this allocation function
+// in all submodules of the current module. To make sure that the other
+// submodules can lookup this function, we u
rsmith added inline comments.
Comment at: include/clang/Serialization/ASTReader.h:551
+ llvm::SmallVector PendingLazySpecializationIDs;
+
I'm not sure we can safely use global state for this: loading update records
can trigger the import of a declaration and
rsmith added a comment.
Counterproposal: in `-std=*++14` onwards, treat this construct as a
user-defined literal, but fall back on the built-in interpretation if name
lookup doesn't find an `operator""i` function. (The two interpretations only
conflict if the source code explicitly does somethi
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D31692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith added inline comments.
Comment at: lib/AST/ItaniumMangle.cpp:2329-2333
+ // __unaligned is not currently mangled in any way. This implies that it is
+ // not a relevant qualifier for substitutions (while CVR and maybe others
+ // are). This triggers an assertion when th
rsmith added a comment.
Do we really need two different notions of "definition" and "odr definition"
here? What goes wrong if we always treat the "instantiation of a friend
function definition" case as being a definition?
Comment at: include/clang/AST/Decl.h:1789
+ // fu
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Does it still make sense for us to have a `UO_Coawait` at all? As I recall, the
only purpose it ever had was to represent a dependent `co_await` expression
that couldn't yet be resolved to a `
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D31608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith added inline comments.
Comment at: include/clang/AST/StmtCXX.h:128
+struct CXXTryStmt::OnStack : CXXTryStmt {
+ alignas(CXXTryStmt) Stmt *Stmts[2];
+ OnStack(SourceLocation tryLoc, Stmt *tryBlock, Stmt *handler)
This makes more assumptions about record l
rsmith added inline comments.
Comment at: lib/CodeGen/CGCoroutine.cpp:273
+
+// FIXME: There must be a cleaner way to do this!
+if (auto *Cleanup =
dyn_cast_or_null(&*CGF.EHStack.begin())) {
It doesn't seem safe to assume that a prior `EHCleanupScope` wo
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:338
+ // In a coroutine, only co_return statements count as normal returns.
Remember
+ // if we are processing the coro
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
https://reviews.llvm.org/D9
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
rsmith added a comment.
This is supposed to be handled by `Sema::DeclareGlobalAllocationFunction`:
DeclContext::lookup_result R = GlobalCtx->lookup(Name);
for (DeclContext::lookup_iterator Alloc = R.begin(), AllocEnd = R.end();
Alloc != AllocEnd; ++Alloc) {
// Only look at non-temp
rsmith added a comment.
In https://reviews.llvm.org/D9#759146, @hubert.reinterpretcast wrote:
> The `check-all` target passes even if the ellipsis-after-declarator-id
> disambiguation as a declarator is removed entirely.
[...]
> So, on the whole, the stray ellipsis treatment is both too c
rsmith added inline comments.
Comment at: include/clang/Parse/Parser.h:1956-1957
AccessSpecifier AS, DeclSpecContext DSC);
- void ParseEnumBody(SourceLocation StartLoc, Decl *TagDecl);
+ void ParseEnumBody(SourceLocation StartLoc, Decl *TagDecl,
+
rsmith added a comment.
Should I assume our "misplaced ellipsis" diagnostic requires that we
disambiguate the ill-formed ellipsis-after-declarator-id as a declaration in
some cases? If so, do we have tests for that somewhere?
Comment at: include/clang/Parse/Parser.h:2138
+ T
rsmith accepted this revision.
rsmith added a comment.
Some comments, but I'm happy for you to go ahead and commit after addressing
them. Thanks!
Comment at: include/clang/Lex/Preprocessor.h:2004
+ ArrayRef getPreambleConditionalStack() const
+ { return PreambleConditionalSt
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
https://reviews.llvm.org/D33207
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
rsmith added inline comments.
Comment at: lib/Parse/ParseExpr.cpp:203-222
+ // Create the ExpressionEvaluationContext on the stack - but only if asked
to.
+ struct EnterExpressionEvaluationContextConditionalRAII {
+llvm::AlignedCharArray
+MyStackStorage;
+const
rsmith added inline comments.
Comment at: test/SemaCXX/warn-shadow.cpp:214
+void handleLinkageSpec() {
+ typedef void externC; // expected-warning {{declaration shadows a typedef in
linkage specification}}
+}
We should be producing a diagnostic talking about th
rsmith added a comment.
Clang's regression test suite is not a sensible home for these tests. We should
have a home and a plan for system-specific integration tests, but this is not
it. Perhaps this should instead be a part of LNT / the test-suite project?
https://reviews.llvm.org/D32178
__
rsmith added a comment.
I'd like to suggest an alternative design: don't add a new attribute., and
instead change the semantics of `__attribute__((overloadable))` to permit at
most one non-overloadable function in an overload set. That one function would
implicitly get the `transparently_overlo
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D33013
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D16171#540261, @phabricss wrote:
> ../include/sys/stat.h:16:15: error: cannot apply asm label to function
> after its first use
> hidden_proto (__fxst
rsmith added a comment.
Er, please ignore the inline review comments; those predated the realisation
that this doesn't actually fix the glibc build problem.
https://reviews.llvm.org/D16171
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
rsmith added a comment.
Thank you, some of these test typos are ... alarming. =)
A couple of the test updates don't look quite right, but this mostly looks
great.
Comment at: test/Driver/amdgpu-features.c:1
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Please first check in a change that just regenerates cxx_dr_status without your
changes, to separate out the changes due to the new issues list from the
changes due to this patch. (You can go
rsmith added inline comments.
Comment at: include/clang/Sema/Sema.h:1464
+ /// Determine if \p D abd \p Suggested have a structurally compatibale
+ /// layout as described in C11 6.2.7/1.
rsmith wrote:
> Typo 'abd'
Typo 'compatibale' =)
Com
rsmith added inline comments.
Comment at: lib/Sema/SemaLookup.cpp:4971-4972
assert(Owner && "definition of hidden declaration is not in a module");
+ assert((!isVisible(Decl) || VisibleModules.isVisible(Owner)) &&
+ "missing import for non-hidden decl?");
--
rsmith added inline comments.
Comment at: lib/Serialization/ASTReaderDecl.cpp:213-215
+ assert(isa(D) ||
+ isa(D) &&
+ "Decl doesn't have specializations.");
Can this ever fail at runtime? I'd expect the below code to not compile if `
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
This makes a lot of sense to me. See also r302463: I think we probably want
three levels of shadowing here: the main input shadows -fmodule-map-file, which
shadows module maps loaded implicitl
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Let's go ahead with this. I've been unable to find a testcase that triggers the
problem, but we shouldn't keep a known latent bug around just because we don't
know how to expose it yet.
http
rsmith added inline comments.
Comment at: test/CXX/drs/dr4xx.cpp:1205
-namespace dr496 { // dr496: no
+namespace dr496 { // dr496: reverted by dr2095 in 5.0
struct A { int n; };
Write this as "dr496: sup 2094" and then rerun the `make_cxx_dr_status` script
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Committed as r301822.
Comment at: lib/AST/ExprConstant.cpp:1301
void addUnsizedArray(EvalInfo &Info, QualType ElemTy) {
- assert(Designator.Entries.empty() && getTy
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Thanks. Could you also add something like:
struct A {};
struct B : virtual A {};
constexpr A &a = (A&)*(B*)0;
to test/SemaCXX/constant-expression-cxx11.cpp to ensure we produce a suitabl
rsmith added a comment.
Thanks, this looks good, just a couple of minor things and then it should be
ready to land.
Do you have commit access or will you need someone else to commit this for you?
Comment at: lib/AST/ExprConstant.cpp:152
+ uint64_t &A
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Yes, let's first revert back to the clang 4.0 behavior, then please mail
cfe-dev to discuss what the right behavior should be.
Repository:
rL LLVM
https://reviews.llvm.org/D32566
__
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
https://reviews.llvm.org/D32410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: test/Preprocessor/macro_paste_commaext.c:4
+// In the following tests, note that the output is sensitive to the
+// whitespace *preceeding* the varargs argum
rsmith added a comment.
I'm OK with this from a mechanical perspective. But there's also a libclang
design question here: what should the libclang methods to query template
arguments for a type cursor representing an alias template specialization
actually do? Should there be some way for a libc
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D32405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith added inline comments.
Comment at: lib/AST/ExprConstant.cpp:4469-4470
{ return StmtVisitorTy::Visit(E->getSubExpr()); }
+ bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
+{ return StmtVisitorTy::Visit(E->getSubExpr()); }
bool VisitChooseExpr(const ChooseExpr *
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
If we're now catching integer overflow in more cases, please add some relevant
testcases. If this is a pure refactoring that enables those additional
diagnostics to be produced in future, then
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D32455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rsmith added a comment.
This change looks like it introduces a regression itself: given
template struct A {};
template using B = A;
B bi;
... requesting the template arguments for the type `B` changes from
producing `int` to producing `int*` with this patch, which seems to directly
oppo
rsmith added a comment.
The change in direction from diagnosing the lvalue-to-rvalue conversion to
diagnosing the pointer arithmetic seems fine to me (and is likely a better
approach overall), but this means we should now treat a designator referring to
element 0 of an array of unknown / runtim
rsmith added inline comments.
Comment at: lib/Parse/ParseTemplate.cpp:1203-1204
+ {
+EnterExpressionEvaluationContext EnterConstantEvaluated(
+Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+if (isCXXTypeId(TypeIdAsTemplateArgument)) {
--
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM. I'd like to make sure we try to use something better than the first
import location for a module (that location is especially meaningless under
`-fmodules-local-submodule-visibility`), b
rsmith added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:3066-3067
+} else { // Unsigned integers and pointers.
+ if (CGF.CGM.getCodeGenOpts().StrictVTablePointers &&
+ CGF.CGM.getCodeGenOpts().OptimizationLevel > 0) {
+// Based on comparis
rsmith added a comment.
In https://reviews.llvm.org/D31856#733845, @efriedma wrote:
> We normally use stdint.h from the host C library, rather than our own
> version; this file is only relevant in -ffreestanding mode. So it should be
> safe to change.
Agreed; r89237 (and nearby changes: r892
rsmith added inline comments.
Comment at: test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp:46
+std::gcd(static_cast(0), static_cast(0)))>::value, "");
+const bool result = static_cast>(out) ==
+std::gcd(static_cast(in1), static_cast(in2));
--
rsmith added a comment.
This needs testcases (the one from your summary plus the ones in my comments
above would be good).
Comment at: lib/AST/ExprConstant.cpp:2622
// Next subobject is an array element.
- const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayT
rsmith added a comment.
Please add some test coverage for these triples.
Repository:
rL LLVM
https://reviews.llvm.org/D32269
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.
This is sadly not a correct change. The relevant requirements (C11 7.20.3/2) on
these macros are:
> Each instance of these macros shall be replaced by a constant expression
> suitab
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
I'm not thrilled about adding yet more predefined macros, but it really doesn't
make sense for libc++ to depend on `__GCC_*` macros when targeting Windows, nor
for these to be Windows-only, so
rsmith added a comment.
In https://reviews.llvm.org/D32199#732189, @hfinkel wrote:
> In https://reviews.llvm.org/D32199#731472, @rsmith wrote:
>
> > 1. C's "effective type" rule allows writes to set the type pretty much
> > unconditionally, unless the storage is for a variable with a declared ty
rsmith added a comment.
> As I've currently implemented it, both reads and writes set the type of
> previously-unknown storage, and after that it says fixed (unless you memcpy
> to it, memset it, or its lifetime ends (the type gets reset on
> lifetime.start/end and for malloc/allocas/etc.). The
rsmith added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:3850-3853
// In Microsoft mode, prefer an integral conversion to a
// floating-to-integral conversion if the integral conversion
// is between types of the same size.
// For example:
rsmith added inline comments.
Comment at: include/clang/Sema/Sema.h:1464
+ /// Determine if \p D abd \p Suggested have a structurally compatibale
+ /// layout as described in C11 6.2.7/1.
Typo 'abd'
Comment at: lib/Parse/ParseDecl.cpp:4236-
rsmith added a comment.
> ! In https://reviews.llvm.org/D32199#731252, @hfinkel wrote:
>
>> How about renaming this to something more like `-fsanitize=type`?
>
> I'm fine with that. Do you like TypeSanitizer or TypeAccessSantizer or
> TypeAliasingSanitizer best?
I think calling it a type alias
rsmith added a comment.
I don't like calling this a "TBAA sanitizer". What we're sanitizing is the
object model and effective type rules; it seems irrelevant which specific
compiler analysis passes would result in your program misbehaving if you break
the rules. I would also expect that we will
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 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 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.
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
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 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
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 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.
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 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.
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
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.
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
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, 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 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 added inline comments.
Comment at: lib/Sema/Sema.cpp:472-477
+// If this is a function template, we should remove if it has no
+// specializations.
+if (FunctionTemplateDecl *Template = FD->getDescribedFunctionTemplate()) {
+ if (std::distance(Template->sp
rsmith added a comment.
Functionally, this looks good. How do the diagnostics look in the case where
lookup only finds a non-namespace name? Eg,
struct A { struct B {}; };
namespace X = A::B;
https://reviews.llvm.org/D30848
___
cfe-commits mai
rsmith added inline comments.
Comment at: test/CXX/drs/dr3xx.cpp:911
-namespace dr373 { // dr373: no
- // FIXME: This is valid.
- namespace X { int dr373; } // expected-note 2{{here}}
+namespace dr373 { // dr373: yes
+ namespace X { int dr373; }
This should
rsmith added inline comments.
Comment at: lib/Sema/TreeTransform.h:6802
+ return getDerived().RebuildDependentCoawaitExpr(
+ E->getKeywordLoc(), Result.get(), E->getOperatorCoawaitLookup());
+}
EricWF wrote:
> rsmith wrote:
> > You need to transform the Unr
rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
Comment at: include/clang/Sema/ScopeInfo.h:138-140
+ /// \brief Whether this function has already built, or tried to build, the
+ /// the initial and final coroutine su
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Hal and I discussed exactly the same problem (in the context of coroutines) on
Saturday and came up with exactly the same approach :) I think this is the
right direction.
C
rsmith added a comment.
In https://reviews.llvm.org/D29753#688834, @bruno wrote:
> > 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
>
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
___
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.
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
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.
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
1 - 100 of 188 matches
Mail list logo
