aaron.ballman closed this revision.
aaron.ballman added a comment.
Committed in r330159. Thank you for the patch!
https://reviews.llvm.org/D35181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
jklaehn updated this revision to Diff 142579.
jklaehn added a comment.
Thanks for the review! As I do not have commit access, it would be great if
you could commit the updated patch.
https://reviews.llvm.org/D35181
Files:
include/clang/Basic/IdentifierTable.h
lib/Basic/IdentifierTable.cpp
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a few small nits.
Comment at: include/clang/Basic/IdentifierTable.h:471
+ /// \brief Create the identifier table.
+ IdentifierTable(Identifier
jklaehn added a comment.
friendly ping? :)
Repository:
rC Clang
https://reviews.llvm.org/D35181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jklaehn updated this revision to Diff 141033.
jklaehn set the repository for this revision to rC Clang.
jklaehn added a comment.
ping? (rebased)
Repository:
rC Clang
https://reviews.llvm.org/D35181
Files:
include/clang/Basic/IdentifierTable.h
lib/Basic/IdentifierTable.cpp
lib/Lex/Prepr
jklaehn updated this revision to Diff 127685.
jklaehn added a project: clang.
jklaehn added a comment.
ping (rebased)
https://reviews.llvm.org/D35181
Files:
include/clang/Basic/IdentifierTable.h
lib/Basic/IdentifierTable.cpp
lib/Lex/Preprocessor.cpp
unittests/libclang/LibclangTest.cpp
jklaehn added a comment.
In https://reviews.llvm.org/D35181#948925, @rsmith wrote:
> LGTM, but I'd like the old `IdentifierTable` constructor to be removed if
> there are no callers left.
It's still being used in e.g. `FormatTokenLexer`, where the populated
`IdentifierTable` is passed to the
rsmith added a comment.
LGTM, but I'd like the old `IdentifierTable` constructor to be removed if there
are no callers left.
Comment at: include/clang/Basic/IdentifierTable.h:473-476
/// \brief Create the identifier table, populating it with info about the
/// language k
jklaehn added a comment.
Ping, can you take another look?
https://reviews.llvm.org/D35181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jklaehn updated this revision to Diff 123495.
jklaehn added a reviewer: arphaman.
jklaehn added a comment.
Thanks for taking a look! I removed the constructor argument as suggested;
keywords are now added in `PP.Initialize`.
https://reviews.llvm.org/D35181
Files:
include/clang/Basic/Identifi
arphaman added a comment.
Thanks for the patch!
Comment at: lib/Frontend/ASTUnit.cpp:541
+// language.
+PP.getIdentifierTable().AddKeywords(LangOpt);
+
Have you tried adding the keywords in `PP.Initialize`? We might not need an
additional parameter to
jklaehn added a comment.
ping :)
https://reviews.llvm.org/D35181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jklaehn created this revision.
In `ASTUnit::LoadFromASTFile`, the preprocesor object is set up using
default-constructed `LangOptions` (which only later get populated).
Then, in the constructor of `IdentifierTable`, these default-constructed
`LangOptions` were used in the call to `AddKeywords`, le
13 matches
Mail list logo
