cor3ntin added inline comments.

================
Comment at: clang/lib/Sema/SemaExpr.cpp:3586-3591
+  // MSVC treats all predefined expressions as string literals rather than char
+  // arrays.
+  if (LangOpts.MicrosoftExt)
+    return SL;
+
   return PredefinedExpr::Create(Context, Loc, ResTy, IK, SL);
----------------
aaron.ballman wrote:
> aeubanks wrote:
> > aaron.ballman wrote:
> > > This is incorrect -- these are still predefined expressions even if 
> > > they're a string literal. Otherwise, we lose AST fidelity and things like 
> > > the `predefinedExpr()` AST matcher don't work 
> > > (https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L2697),
> > >  and `-ast-print` will print the wrong thing 
> > > (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/StmtPrinter.cpp#L1248).
> > how would you structure this? an almost identical `PredefinedExpr` class 
> > that also subclasses `StringLiteral`? or special case all the places that 
> > check for `StringLiteral` to also check for `PredefinedExpr` (the case I 
> > was mostly looking at was initializing a char array with predefined 
> > expressions but presumably there are more places that matter)
> We can't inherit from `StringLiteral` (it's a `final` class because it has 
> `TrailingObjects`), so that might be the only viable approach. However, 
> there's a fair number of places in the compiler where we care if something is 
> or isn't a `StringLiteral`, so I can see why we'd want to avoid that if 
> possible.
> 
> Oye, but it seems that even MSVC is not consistent with whether the 
> predefined expression is a string literal or not: 
> https://godbolt.org/z/jzY8cz79d
I think these should be plain old string literals, but for the purpose or AST 
matchers and such, maybe we could have a "predefined macro" bit, or even an 
additional object that stores the name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146764/new/

https://reviews.llvm.org/D146764

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATC... Arthur Eubanks via Phabricator via cfe-commits
    • ... Hans Wennborg via Phabricator via cfe-commits
    • ... Aaron Ballman via Phabricator via cfe-commits
    • ... Martin Storsj√∂ via Phabricator via cfe-commits
    • ... Arthur Eubanks via Phabricator via cfe-commits
    • ... Aaron Ballman via Phabricator via cfe-commits
    • ... Corentin Jabot via Phabricator via cfe-commits
    • ... Arthur Eubanks (out until mid-April) via Phabricator via cfe-commits
    • ... Richard Smith - zygoloid via Phabricator via cfe-commits
    • ... Aaron Ballman via Phabricator via cfe-commits

Reply via email to