- User Since
- Jun 10 2018, 8:27 AM (32 w, 2 d)
Some small additional remarks if you are already modifying this class.
Sun, Jan 20
Sat, Jan 19
Fri, Jan 18
This is not worth doing.
The StringLiteral::setString interface has been removed in D54166 so I am closing this revision.
Wed, Jan 16
Wed, Jan 9
This commit made the reverse-iteration bot (http://lab.llvm.org:8011/builders/reverse-iteration) red (red since 6 months now) :
Tue, Jan 8
I have no more remark, but since I am just a new contributor to clang I am sure
people will have more to say about this patch. Thanks !
Mon, Jan 7
Okay. That comment seems reasonable. Glad to hear you're on top of it. :)
I guess that means I can land this as is ?
Sun, Jan 6
Friendly ping ?
Sat, Jan 5
Fri, Dec 28
Dec 22 2018
Dec 21 2018
Dec 20 2018
Is anyone working on this ? This is the only persistent msan failure when doing check-clang on my machine.
Dec 19 2018
Yes, I was thinking about the same thing! However, I believe that some people may find this kind of "redundant" access specs actually more readable, so maybe it makes sense to add a check option for this > to allow users to disable it?
Dec 18 2018
Make OffsetToTrailingObjects byte sized and aligned to a byte multiple.
Dec 17 2018
Do you really have to add an iterator for this particular thing ?
Why not just use specific_decl_iterator<AccessSpecDecl> in you clang-tidy checker ?
Abandoned in favor of D55771
Exactly. And this seems to be an important feature in ASTImporter, because this makes it possible that we can chain into the redecl chain a newly imported AST node to existing and structurally equivalent > nodes. (The existing nodes are found by the lookup and then we iterate over the lookup results searching for structurally equivalent ones.)
This is a perhaps a naive comment, but why is localUncachedLookup used
instead of noload_lookup ? There is a fixme dating from 2013 stating
that noload_lookup should be used instead.
Dec 15 2018
Dec 14 2018
Addressed inline comments
Just to expend on the instrumentation results: The measurement was done with all of Boost. I would take the estimation of the time wasted with
a grain of salt since this is just num_iterations * 10ns which is obviously a very rough estimation.
However on my machine I get that removing half of the iterations in getTranslationUnitDecl reduce the run-time of an fsyntax-only by a little more than 1%.
Dec 13 2018
Dec 12 2018
Dec 11 2018
Dec 9 2018
Dec 5 2018
Dec 4 2018
Can this go in now that D54547 has been committed ?
Dec 3 2018
Forgot a space in the error message
Dec 1 2018
Much better. LGTM with a small format nit.
Nov 30 2018
Nov 28 2018
And moreover I believe this change is subtly incorrect for the following reason:
The type that is passed into the constructor of the call expression is the type
of the call expression, and not the return type.
Nov 27 2018
I see plenty of TheCall->setType left in Sema::CheckBuiltinFunctionCall
Builtin::BI__builtin_dump_struct and so on...).
added inline comments
fixed the formatting
Nov 26 2018
Nov 23 2018
I looked at how ASTReader create the IdentifierInfos returned by IdentifierInfo *ASTReader::get(StringRef Name),
and I ended up in ASTIdentifierLookupTrait::ReadData, which calls among other things IdentifierTable::getOwn.
The IdentifierInfo created by getOwn will always have the Entry pointer set and therefore I think it is
possible to remove the if (Entry) in IdentifierInfo::getLength and IdentifierInfo::getNameStart.