riccibruno added inline comments.
================
Comment at: include/clang/AST/Expr.h:4108
+public:
+ enum IdentType { Function, File, Line, Column };
+
----------------
`IdentType` -> `IdentKind` ?
================
Comment at: include/clang/AST/Expr.h:4147
+ SourceLocation getBeginLoc() const LLVM_READONLY { return BuiltinLoc; }
+ SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+
----------------
I don't think that `LLVM_READONLY` is useful here since it is just a simple
getter.
Same remark for `getIdentType`, `isStringType` and `isIntType`.
And even for `getBuiltinStr` does it actually make a difference ?
================
Comment at: include/clang/AST/Stmt.h:657
+ class SourceLocExprBitfields {
+ friend class SourceLocExpr;
----------------
Could you please put this in the same place as in
`StmtNodes.td` ? (ie just after `PseudoObjectExpr`)
At least not under "C++ coroutines TS bitfields classes"...
================
Comment at: lib/AST/ASTContext.cpp:10145
+ unsigned Length) const {
+ // A C++ string literal has a const-qualified element type (C++ 2.13.4p1).
+ EltTy = EltTy.withConst();
----------------
And what about C ? It seems to me that `getStringLiteralArrayType`
should be used systematically when the type of a string literal is needed.
(eg in `ActOnStringLiteral` but this is not the only place...)
================
Comment at: lib/AST/Expr.cpp:1961
+ // A C++ string literal has a const-qualified element type (C++ 2.13.4p1).
+ if (Ctx.getLangOpts().CPlusPlus || Ctx.getLangOpts().ConstStrings)
+ Ty = Ty.withConst();
----------------
Is it taking into account `adjustStringLiteralBaseType` as in
`getStringLiteralArrayType` ?
================
Comment at: lib/AST/Expr.cpp:1978
+ BuiltinLoc(BLoc), RParenLoc(RParenLoc), ParentContext(ParentContext) {
+ SourceLocExprBits.Type = Type;
+}
----------------
The name `Type` here is really unfortunate imho.
================
Comment at: lib/AST/Expr.cpp:1992
+ }
+}
+
----------------
`llvm_unreachable("unexpected IdentType!")`
================
Comment at: lib/AST/Expr.cpp:2025
+ // If we have a simple identifier there is no need to cache the
+ // human readable name.
+ if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo())
----------------
This comment is strange since MkStr will call
`getPredefinedStringLiteralFromCache`
when `FD->getDeclName()` is a simple identifier.
================
Comment at: lib/AST/Expr.cpp:2037
+ llvm::APSInt IntVal(
+ llvm::APInt(Ctx.getTargetInfo().getIntWidth(), LineOrCol));
+ return APValue(IntVal);
----------------
Both `getLine`, `getColumn` and `APInt` take an unsigned.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37035/new/
https://reviews.llvm.org/D37035
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
