Committed as r331552.
On Fri, May 4, 2018 at 12:43 PM, Mikhail Ramalho via Phabricator <
revi...@reviews.llvm.org> wrote:
> mikhail.ramalho updated this revision to Diff 145255.
> mikhail.ramalho added a comment.
>
> Fixed the test case.
>
>
> https://reviews.llvm.org/D36610
>
> Files:
>
mikhail.ramalho updated this revision to Diff 145255.
mikhail.ramalho added a comment.
Fixed the test case.
https://reviews.llvm.org/D36610
Files:
include/clang/AST/QualTypeNames.h
lib/AST/QualTypeNames.cpp
unittests/Tooling/QualTypeNamesTest.cpp
Index:
I applied this to a clean local copy, which has no other changes, and have
the following test error, which may be pilot error on my part, but
nevertheless, this test needs to be robust to changes in the line number.
llvm-svn/llvm/tools/clang/unittests/Tooling/QualTypeNamesTest.cpp:39:
Failure
mikhail.ramalho added a comment.
Thanks.
I just need someone to push it, as I don't have write access to the repo.
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
saugustine added a comment.
In https://reviews.llvm.org/D36610#1083952, @mikhail.ramalho wrote:
> Ping.
Given that richard smith is the only non-approver, and that he hasn't
responded, and that I contributed this code, I'm going to make an executive
decision and say that this is OK to
Last review asked to change the test case, I just want to make sure
everything is fine this time.
2018-05-02 8:54 GMT+01:00 Manuel Klimek via Phabricator <
revi...@reviews.llvm.org>:
> klimek added a comment.
>
> I believe this was accepted by rnk - are you waiting for specific further
>
mikhail.ramalho added subscribers: rsmith, rnk.
mikhail.ramalho added a comment.
Last review asked to change the test case, I just want to make sure
everything is fine this time.
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
klimek added a comment.
I believe this was accepted by rnk - are you waiting for specific further
feedback?
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
mikhail.ramalho marked an inline comment as done.
mikhail.ramalho added a comment.
Ping.
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mikhail.ramalho marked an inline comment as done.
mikhail.ramalho added a comment.
ping.
Comment at: unittests/Tooling/QualTypeNamesTest.cpp:225
+
+ TypeNameVisitor PrintingPolicy;
+ PrintingPolicy.ExpectedQualTypeNames["a"] = "short";
rnk wrote:
> Please
mikhail.ramalho updated this revision to Diff 142709.
mikhail.ramalho added a comment.
Renamed variables in the test so it doesn't match the type name
https://reviews.llvm.org/D36610
Files:
include/clang/AST/QualTypeNames.h
lib/AST/QualTypeNames.cpp
rnk accepted this revision.
rnk added a comment.
Mostly looks good.
Comment at: unittests/Tooling/QualTypeNamesTest.cpp:225
+
+ TypeNameVisitor PrintingPolicy;
+ PrintingPolicy.ExpectedQualTypeNames["a"] = "short";
Please choose a different variable name
mikhail.ramalho updated this revision to Diff 142578.
mikhail.ramalho added a comment.
Rebased to current master.
Removed the getFullyQualifiedName overloaded function.
https://reviews.llvm.org/D36610
Files:
include/clang/AST/QualTypeNames.h
lib/AST/QualTypeNames.cpp
mikhail.ramalho added a comment.
Should we keep just the version with the custom printing policy then?
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
klimek added a comment.
Sorry for jumping in late, but I'm somewhat questioning whether an extra
function in the API is worth the cost here:
If you already have a printing policy, getting the type name will be 1 line
instead of 2; having an extra function in a large API seems not worth it at a
mikhail.ramalho marked an inline comment as done.
mikhail.ramalho added a comment.
ping
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mikhail.ramalho updated this revision to Diff 63.
mikhail.ramalho added a comment.
Updated QualTypeNamesTest.cpp to use multiline string in all tests.
Also ran clang-format -style=LLVM for code style.
https://reviews.llvm.org/D36610
Files:
include/clang/Tooling/Core/QualTypeNames.h
mikhail.ramalho added a comment.
Should I update the other tests in QualTypeNamesTest.cpp to use multiline
strings as well?
https://reviews.llvm.org/D36610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
saugustine accepted this revision.
saugustine added a comment.
This revision is now accepted and ready to land.
This is a good change as far as functionality, but I defer to others on the
style and other details.
https://reviews.llvm.org/D36610
arphaman added inline comments.
Comment at: unittests/Tooling/QualTypeNamesTest.cpp:247
+ "struct (anonymous struct at input.cc:1:1)";
+ PrintingPolicy.runOver(
+ "struct\n"
You should probably use multiline string here, i.e. R(" ... ")
mikhail.ramalho created this revision.
Herald added a subscriber: klimek.
In our user case, we need to generate unique names for every tag, including
anonymous structs/unions.
This adds a custom PrintingPolicy option to getFullyQualifiedName and the test
case shows that different names are
21 matches
Mail list logo
