abhina.sreeskantharajan marked 3 inline comments as done.
abhina.sreeskantharajan added inline comments.
================
Comment at: clang/include/clang/Lex/LiteralTranslator.h:30-31
+
+class LiteralTranslator {
+public:
+ llvm::StringRef InternalCharset;
----------------
tahonermann wrote:
> I don't know the LLVM style guides well, but I suspect a class with all
> public members should be defined using `struct` and not include access
> specifiers.
I've made these private.
================
Comment at: clang/include/clang/Lex/LiteralTranslator.h:37
+
+ llvm::CharSetConverter *getConversionTable(const char *Codepage);
+ CharsetTableStatusCode findOrCreateExecCharsetTable(const char *To);
----------------
tahonermann wrote:
> `getConversionTable()` is logically `const`. Perhaps `ExecCharsetTables`
> should be `mutable`.
>
> From a terminology stand point, this function is misnamed. It doesn't return
> a table, it returns a converter for an encoding. I suggest:
> llvm::CharSetConverter *getCharSetConverter(const char *Encoding) const;
I've renamed this function to getConverter
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93031/new/
https://reviews.llvm.org/D93031
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
