Update for review

Author: rintaro

include/swift/IDETool/SyntacticMacroExpansion.h

@@ -56,7 +56,7 @@ class SyntacticMacroExpansionInstance {
                           const MacroExpansionSpecifier &expansion);
 
   /// Expand single 'expansion' in SF.
-  bool getExpansion(swift::SourceFile *SF,
+  void expand(swift::SourceFile *SF,
                     const MacroExpansionSpecifier &expansion,
                     SourceEditConsumer &consumer);
 
@@ -71,7 +71,7 @@ class SyntacticMacroExpansionInstance {
 
   /// Expand all macros in \p inputBuf and send the edit results to \p consumer.
   /// Expansions are specified by \p expansions .
-  bool getExpansions(llvm::MemoryBuffer *inputBuf,
+  void expandAll(llvm::MemoryBuffer *inputBuf,
                      ArrayRef<MacroExpansionSpecifier> expansions,
                      SourceEditConsumer &consumer);
 };

lib/IDETool/SyntacticMacroExpansion.cpp

@@ -219,7 +219,7 @@ expandFreestandingMacro(MacroDecl *macro,
   std::vector<unsigned> bufferIDs;
 
   SmallString<32> discriminator;
-  discriminator.append("macro_");
+  discriminator.append("__syntactic_macro_");
   addExpansionDiscriminator(discriminator,
                             expansion->getDeclContext()->getParentSourceFile(),
                             expansion->getPoundLoc());
@@ -293,7 +293,9 @@ static Identifier getCustomAttrName(ASTContext &ctx, const CustomAttr *attr) {
   }
 
   // If the attribute is not an identifier type, create an identifier with its
-  // textual representation.
+  // textual representation. This is *not* expected to be reachable.
+  // The only case is like `@Foo?` where the client should not send the
+  // expansion request on this in the first place.
   SmallString<32> name;
   llvm::raw_svector_ostream OS(name);
   tyR->print(OS);
@@ -406,7 +408,7 @@ class MacroExpansionFinder : public ASTWalker {
 };
 } // namespace
 
-bool SyntacticMacroExpansionInstance::getExpansion(
+void SyntacticMacroExpansionInstance::expand(
     SourceFile *SF, const MacroExpansionSpecifier &expansion,
     SourceEditConsumer &consumer) {
 
@@ -417,7 +419,7 @@ bool SyntacticMacroExpansionInstance::getExpansion(
   SF->walk(expansionFinder);
   auto expansionNode = expansionFinder.getResult();
   if (!expansionNode)
-    return true;
+    return;
 
   // Expand the macro.
   std::vector<unsigned> bufferIDs;
@@ -454,20 +456,16 @@ bool SyntacticMacroExpansionInstance::getExpansion(
                                         /*adjust=*/false,
                                         /*includeBufferName=*/false);
   }
-
-  return false;
 }
 
-bool SyntacticMacroExpansionInstance::getExpansions(
+void SyntacticMacroExpansionInstance::expandAll(
     llvm::MemoryBuffer *inputBuf, ArrayRef<MacroExpansionSpecifier> expansions,
     SourceEditConsumer &consumer) {
 
   // Create a source file.
   SourceFile *SF = getSourceFile(inputBuf);
 
-  bool hasError = false;
   for (const auto &expansion : expansions) {
-    hasError |= getExpansion(SF, expansion, consumer);
+    expand(SF, expansion, consumer);
   }
-  return hasError;
 }

test/SourceKit/Macros/syntactic_expansion.swift.expected

@@ -29,53 +29,53 @@ source.edit.kind.active:
   10:3-10:3 "@Wrapper"
 source.edit.kind.active:
   12:3-12:35 "struct blah8 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu0_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu0_() {
   }
 }
 struct blah16 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu1_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu1_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu2_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu2_() {
   }
 }
 struct blah32 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu3_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu3_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu4_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu4_() {
   }
 }
 struct blah64 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu5_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu5_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu6_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu6_() {
   }
 }"
 ### 2
 source.edit.kind.active:
   12:3-12:35 "struct blah8 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu0_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu0_() {
   }
 }
 struct blah16 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu1_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu1_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu2_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu2_() {
   }
 }
 struct blah32 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu3_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu3_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu4_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu4_() {
   }
 }
 struct blah64 {
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu5_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu5_() {
   }
-  func macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu6_() {
+  func __syntactic_macro_a01f07de3e9ee8585adf794ccd6dec8c6methodfMu6_() {
   }
 }"
 source.edit.kind.active:

tools/SourceKit/include/SourceKit/Core/LangSupport.h

@@ -244,6 +244,9 @@ struct MacroExpansionInfo {
   };
   // See swift::ExpandedMacroDefinition.
   struct ExpandedMacroDefinition {
+    // 'Replacement.range' references some part of code in 'expansionText'.
+    // 'expansionText' will be replaced by the 'parameterIndex'-th argument of
+    // the macro.
     struct Replacement {
       RawCharSourceRange range;
       unsigned parameterIndex;
@@ -257,7 +260,8 @@ struct MacroExpansionInfo {
         : expansionText(expansionText), replacements(){};
   };
 
-  // Offset of the macro expansion syntax (i.e. attribute or #<macro name>)
+  // Offset of the macro expansion syntax (i.e. attribute or #<macro name>) from
+  // the start of the source file.
   unsigned offset;
 
   // Macro roles.

tools/SourceKit/lib/SwiftLang/SwiftSyntacticMacroExpansion.cpp

@@ -89,6 +89,6 @@ void SwiftLangSupport::expandMacroSyntactically(
   }
 
   RequestRefactoringEditConsumer consumer(receiver);
-  (void)instance->getExpansions(inputBuf, expansions, consumer);
+  instance->expandAll(inputBuf, expansions, consumer);
   // consumer automatically send the results on destruction.
 }

tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

@@ -422,10 +422,10 @@ static bool readPopularAPIList(StringRef filename,
 /// where {path} is a path to a JSON file that has macro roles and definition.
 /// {line} and {column} is resolved to 'offset' using \p inputBuf .
 static bool setSyntacticMacroExpansions(sourcekitd_object_t req,
-                                        TestOptions &Opts,
+                                        TestOptions &opts,
                                         llvm::MemoryBuffer *inputBuf) {
   SmallVector<sourcekitd_object_t, 4> expansions;
-  for (std::string &opt : Opts.RequestOptions) {
+  for (std::string &opt : opts.RequestOptions) {
     SmallVector<StringRef, 3> args;
     StringRef(opt).split(args, ":");
     unsigned line, column;
@@ -437,7 +437,7 @@ static bool setSyntacticMacroExpansions(sourcekitd_object_t req,
     }
     unsigned offset = resolveFromLineCol(line, column, inputBuf);
 
-    auto Buffer = getBufferForFilename(args[2], Opts.VFSFiles)->getBuffer();
+    auto Buffer = getBufferForFilename(args[2], opts.VFSFiles)->getBuffer();
     char *Err = nullptr;
     auto expansion = sourcekitd_request_create_from_yaml(Buffer.data(), &Err);
     if (!expansion) {
@@ -1150,7 +1150,7 @@ static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) {
 
   case SourceKitRequest::SyntacticMacroExpansion:
     sourcekitd_request_dictionary_set_uid(Req, KeyRequest,
-                                          RequestExpandMacroSyntactically);
+                                          RequestSyntacticMacroExpansion);
     setSyntacticMacroExpansions(Req, Opts, SourceBuf.get());
     break;
   }

tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp

@@ -1845,7 +1845,7 @@ handleRequestDiagnostics(const RequestDict &Req,
 /// Sends the results as a 'CategorizedEdits'. 
 /// Note that, unlike refactoring, each edit doesn't have 'key.buffer_name'.
 /// FIXME: Support nested expansion.
-static void handleRequestExpandMacroSyntactically(
+static void handleRequestSyntacticMacroExpansion(
     const RequestDict &req, SourceKitCancellationToken cancellationToken,
     ResponseReceiver rec) {
 
@@ -2055,8 +2055,8 @@ void handleRequestImpl(sourcekitd_object_t ReqObj,
   HANDLE_REQUEST(RequestRelatedIdents, handleRequestRelatedIdents)
   HANDLE_REQUEST(RequestActiveRegions, handleRequestActiveRegions)
   HANDLE_REQUEST(RequestDiagnostics, handleRequestDiagnostics)
-  HANDLE_REQUEST(RequestExpandMacroSyntactically,
-                 handleRequestExpandMacroSyntactically)
+  HANDLE_REQUEST(RequestSyntacticMacroExpansion,
+                 handleRequestSyntacticMacroExpansion)
 
   {
     SmallString<64> ErrBuf;

utils/gyb_sourcekit_support/UIDs.py

@@ -276,12 +276,9 @@ def __init__(self, internal_name, external_name):
     REQUEST('Diagnostics', 'source.request.diagnostics'),
     REQUEST('Compile', 'source.request.compile'),
     REQUEST('CompileClose', 'source.request.compile.close'),
-<<<<<<< HEAD
     REQUEST('EnableRequestBarriers', 'source.request.enable_request_barriers'),
-=======
-    REQUEST('ExpandMacroSyntactically',
-            'source.request.expand.macro.syntactically'),
->>>>>>> 309c87b56fa ([SourceKit] Add request to expand macros syntactically)
+    REQUEST('SyntacticMacroExpansion',
+            'source.request.syntactic_macro_expansion'),
 ]