[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:1494-1505
+Decl *Sema::getCurLocalScopeDecl() {
+  if (const BlockScopeInfo *BSI = getCurBlock())
+    return BSI->TheDecl;
+  else if (const LambdaScopeInfo *LSI = getCurLambda())
+    return LSI->CallOperator;
+  else if (const CapturedRegionScopeInfo *CSI = getCurCapturedRegion())
+    return CSI->TheCapturedDecl;
----------------
Made the function `const` because it's not writing to any fields, removed `else` because of preceding `return`.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:1986
+  // expansion of macros into empty string literals without additional checks.
+  Decl *currentDecl = getCurLocalScopeDecl();
+  if (!currentDecl)
----------------
Matching our usual naming conventions.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:1998-2000
+    if (isa<TranslationUnitDecl>(currentDecl)) {
+      Diag(Tok.getLocation(), diag::ext_predef_outside_function);
+    }
----------------
This will miss the diagnostic if the current declaration is a namespace instead of at file scope, right?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:2034-2036
+  std::vector<Token> ExpandedToks;
+  if (getLangOpts().MicrosoftExt)
+    StringToks = ExpandedToks = ExpandFunctionLocalPredefinedMacros(StringToks);
----------------
Probably worth a comment here explaining that `ExpandedToks` acts as the backing store for `StringToks`, which is why both are being assigned to here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153914