- User Since
- Jun 10 2018, 8:27 AM (83 w, 5 d)
Fri, Jan 10
Thu, Jan 9
Am I the only one who is a little bit uncomfortable with including a replacement to the system allocator in LLVM? It seems to me that this is something that is clearly under the responsibility of the C library. Now this might still be worth doing given the large performance improvement, but yuck...
Wed, Jan 8
Fri, Jan 3
Thu, Jan 2
Mon, Dec 30
Sun, Dec 29
Not a comment on this change in particular, but I am wondering if this change was motivated by the performance implications of getASTContext() ?
DeclBase::getASTContext() is widely used because it looks cheap, but in fact it is not cheap at all since it does one of the slowest thing possible (following a linked-list of pointers).
Sun, Dec 22
Fri, Dec 20
@rsmith Could you take a look at this patch when you have some time? This is the last C++17 sequencing rule which is missing from SequenceChecker and is very similar to the other already accepted patches.
Thu, Dec 19
These are not the only AST nodes representing cast expressions (there is also CXXFunctionalCastExpr, ...). What about replacing the IgnoreParenImpCasts() above by IgnoreParenCasts() ? Incidentally doing this uncovers another test (Parser/cxx-ambig-decl-expr.cpp ) where this diagnostic is triggered.
Dec 18 2019
Add missing patch context
Dec 17 2019
Note that I am not entirely sure that the implementation in getMemoryLocation/MemoryLocation is the right way to do this; I think that this patch should be considered a work-in-progress.
I have factored out various NFCs which were present in this patch. This should make review easier.
Also addressed some inline comments.
Rebased on top of current master and D58297. Friendly ping !
Rebased on top of current master and D57747. No need to look at it.
Rebased on top of current master and D57659. No need to look at it.
Rebased on top of current master. No need to look at it.
Dec 12 2019
Aug 27 2019
Aug 25 2019
Also test that DeclInfo is trivially destructible.
I was looking at SpecificBumpPtrAllocator, which knows it's type.
But if i look down the indirection, i think AllocatorBase::Allocate<T>()/AllocatorBase::Deallocate<T>() is the place.
Aug 23 2019
Aug 22 2019
This should not warn. Please verify that patch was applied correctly and you use newly built clang with this patch. (I use arc patch DXXXXX)
Aug 21 2019
Is the test up-to-date ? I tried to apply the patch locally and I get a bunch of failures :
Aug 20 2019
Can you submit the patch with the full context (ie: git diff -U9999999) ?
Rebased. Thanks for the review @xbolva00 !
Aug 13 2019
I think that you have to run check-clang-tools. I too was surprised that it was not included in check-clang.
Aug 12 2019
It seems that these two options are not exactly the same right ? The ContainsError bit is useful to quickly answer "Does this expression contains an invalid sub-expression" without doing the search, while adding an ErrorExpr node is useful to note that this sub-expression is invalid (and as Aaron says the hypothetical ErrorExpr node can carry more info about the error).
Jul 27 2019
May 15 2019
May 8 2019
Address Aaron's comments.
May 2 2019
May 1 2019
Already done in r359702 :)
Apr 30 2019
Apr 29 2019
@rsmith Do you have an opinion on whether this ADL rule should be implemented ?
Is it intentional to warn even if all the cases are covered ?
Since this is target-specific, is it right to put this here ?
Apr 25 2019
Apr 23 2019
Closing this since the issue was fixed in r358674.
Added a test which exposes a new problem that this patch incidentally solves (see N_conflicting_namespace_alias). Because of the using directive using namespace M;, the namespace alias i for the namespace Q is found in the redeclaration lookup. Before this patch, since getAsSingle used internally getUnderlyingDecl(), PrevDecl was for the NamespaceDecl for Q, and not for the VarDecl for Q::i. Then the declaration for the enumerator i was mistakenly rejected since Q is in the same scope.
- Added a test (see N_shadow) for the behavior of Wshadow. This test showed that I forgot to change CheckShadow(New, PrevDecl, R); to CheckShadow(New, PrevDecl->getUnderlyingDecl(), R); to match change in the condition of the if statement.
Apr 22 2019
Apr 21 2019
I will take a look at this tomorrow, I know that it is annoying to get no feedback!
@Quuxplusone Do you have other objections apart from the template-id issue ? If not, since D60573 depends on this patch, I would like to commit this with a comment explaining the issue instead of the FIXME.
This looks reasonable to me (although I think that the test should be in SemaCXX/ and not in Parser/, but the test for the non-template case is already in Parser/ so ok).
Apr 20 2019
This patch certainly took a few hours to write. Can you spend 5 minutes on making the summary readable (spelling, capitalization, formatting, well-formed sentences, ...) ?