Merge pull request #1546 from xlsynth:cdleary/2024-08-18-invoke-param…

…etric-non-pub

PiperOrigin-RevId: 665011741

xls/dslx/frontend/ast.h

@@ -35,7 +35,6 @@
 #include "absl/strings/str_format.h"
 #include "absl/types/span.h"
 #include "absl/types/variant.h"
-#include "xls/common/casts.h"
 #include "xls/common/status/status_macros.h"
 #include "xls/dslx/channel_direction.h"
 #include "xls/dslx/frontend/ast_node.h"  // IWYU pragma: export

xls/dslx/frontend/parser.cc

@@ -2351,7 +2351,7 @@ absl::StatusOr<Function*> Parser::ParseProcNext(
 // type.
 absl::StatusOr<Function*> Parser::ParseProcInit(
     Bindings& bindings, std::vector<ParametricBinding*> parametric_bindings,
-    std::string_view proc_name) {
+    std::string_view proc_name, bool is_public) {
   Bindings inner_bindings(&bindings);
   XLS_ASSIGN_OR_RETURN(Token init_identifier, PopToken());
   if (!init_identifier.IsIdentifier("init")) {
@@ -2369,7 +2369,7 @@ absl::StatusOr<Function*> Parser::ParseProcInit(
   Function* init = module_->Make<Function>(
       span, name_def, std::move(parametric_bindings), std::vector<Param*>(),
       /*return_type=*/nullptr, body, FunctionTag::kProcInit,
-      /*is_public=*/false);
+      /*is_public=*/is_public);
   name_def->set_definer(init);
   return init;
 }
@@ -2494,7 +2494,7 @@ absl::StatusOr<T*> Parser::ParseProcLike(bool is_public,
 
       XLS_ASSIGN_OR_RETURN(Function * init,
                            ParseProcInit(member_bindings, parametric_bindings,
-                                         name_def->identifier()));
+                                         name_def->identifier(), is_public));
       proc_like_body.init = init;
       XLS_RETURN_IF_ERROR(module_->AddTop(init, MakeModuleTopCollisionError));
 

xls/dslx/frontend/parser.h

@@ -593,7 +593,7 @@ class Parser : public TokenParser {
 
   absl::StatusOr<Function*> ParseProcInit(
       Bindings& bindings, std::vector<ParametricBinding*> parametric_bindings,
-      std::string_view proc_name);
+      std::string_view proc_name, bool is_public);
 
   // Parses a proc-like entity (i.e. either a Proc or a Block).
   template <typename T>

xls/dslx/tests/errors/error_modules_test.py

@@ -1203,6 +1203,14 @@ def test_nominal_type_mismatch(self):
         'vs .*/dslx/tests/errors/mod_simple_struct_duplicate.x:MyStruct {}\n',
     )
 
+  def test_imports_and_calls_private_parametric(self):
+    stderr = self._run(
+        'xls/dslx/tests/errors/imports_and_calls_private_parametric.x'
+    )
+    self.assertIn(
+        'Attempted to resolve a module member that was not public', stderr
+    )
+
 
 if __name__ == '__main__':
   test_base.main()

xls/dslx/tests/errors/imports_and_calls_private_parametric.x

@@ -0,0 +1,24 @@
+// Copyright 2024 The XLS Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import xls.dslx.tests.errors.mod_private_parametric;
+
+fn main() -> u32 {
+    mod_private_parametric::p<u32:32>()
+}
+
+#[test]
+fn test_main() {
+    assert_eq(main(), u32:0)
+}

xls/dslx/tests/errors/mod_private_parametric.x

@@ -0,0 +1,18 @@
+// Copyright 2024 The XLS Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Note: this does not have a pub annotation.
+fn p<N: u32>() -> bits[N] {
+    bits[N]:0
+}

xls/dslx/type_system/deduce_invocation.cc

@@ -169,8 +169,18 @@ static absl::StatusOr<Function*> ResolveColonRefToFnForInvocation(
       ctx->type_info()->GetImported(*import);
   XLS_RET_CHECK(imported_info.has_value());
   Module* module = imported_info.value()->module;
-  return GetMemberOrTypeInferenceError<Function>(module, ref->attr(),
-                                                 ref->span());
+  XLS_ASSIGN_OR_RETURN(Function * resolved,
+                       GetMemberOrTypeInferenceError<Function>(
+                           module, ref->attr(), ref->span()));
+  if (!resolved->is_public()) {
+    return TypeInferenceErrorStatus(
+        ref->span(), nullptr,
+        absl::StrFormat("Attempted to resolve a module member that was not "
+                        "public; `%s` defined in module `%s` @ %s",
+                        resolved->identifier(), module->name(),
+                        resolved->span().ToString()));
+  }
+  return resolved;
 }
 
 absl::StatusOr<TypeAndParametricEnv> DeduceInstantiation(

xls/examples/fp32_fmac.x

@@ -25,7 +25,6 @@ proc fp32_fmac {
     config(input_a: chan<F32> in, input_b: chan<F32> in,
           reset: chan<bool> in, output: chan<F32> out) {
         spawn apfloat_fmac::fmac<u32:8, u32:23>(input_a, input_b, reset, output);
-        ()
     }
 
     // Nothing to do here - the spawned fmac does all the lifting.

xls/examples/lfsr.x

@@ -20,7 +20,8 @@
 // in a 5-bit LFSR with taps on bits 2 and 4.
 ////////////////////////////////////////////////////////////////////////////////
 
-fn lfsr<BIT_WIDTH: u32>(current_value: uN[BIT_WIDTH], tap_mask: uN[BIT_WIDTH]) -> uN[BIT_WIDTH] {
+pub fn lfsr<BIT_WIDTH: u32>
+    (current_value: uN[BIT_WIDTH], tap_mask: uN[BIT_WIDTH]) -> uN[BIT_WIDTH] {
     // Compute the new bit from the taps
     let new_bit = for (index, xor_bit): (u32, u1) in range(u32:0, BIT_WIDTH) {
         if tap_mask[index+:u1] == u1:0 { xor_bit } else { xor_bit ^ current_value[index+:u1] }

xls/examples/ram.x

@@ -192,7 +192,7 @@ pub fn num_partitions(word_partition_size: u32, data_width: u32) -> u32 {
 //  ASSERT_VALID_READ: if true, add assertion that read operations are only
 //   performed on values that a previous write has set. This is meant to model
 //   asserting that a user doesn't read X from an unitialized SRAM.
-proc RamModel<DATA_WIDTH:u32, SIZE:u32, WORD_PARTITION_SIZE:u32={u32:0},
+pub proc RamModel<DATA_WIDTH:u32, SIZE:u32, WORD_PARTITION_SIZE:u32={u32:0},
   SIMULTANEOUS_READ_WRITE_BEHAVIOR:SimultaneousReadWriteBehavior=
    {SimultaneousReadWriteBehavior::READ_BEFORE_WRITE},
   INITIALIZED:bool={false},
@@ -503,7 +503,7 @@ pub struct RWRamResp<DATA_WIDTH:u32> {
 }
 
 // Models a single-port RAM.
-proc SinglePortRamModel<DATA_WIDTH:u32, SIZE:u32,
+pub proc SinglePortRamModel<DATA_WIDTH:u32, SIZE:u32,
  WORD_PARTITION_SIZE:u32={u32:0},
  ADDR_WIDTH:u32={std::clog2(SIZE)},
  NUM_PARTITIONS:u32={num_partitions(WORD_PARTITION_SIZE, DATA_WIDTH)}> {

xls/examples/sobel_filter.x

@@ -40,7 +40,7 @@ const Y_STENCIL_F32 = map(Y_STENCIL, convert_triplet);
 
 // Apply a stencil 'stencil' on the image 'in_img' to calculate the pixel
 // at row 'row_idx', column 'col_idx' in the output image.
-fn apply_stencil_float32<NUM_ROWS:u32, NUM_COLS:u32, NUM_ELMS:u32 = {NUM_ROWS*NUM_COLS}>(
+pub fn apply_stencil_float32<NUM_ROWS:u32, NUM_COLS:u32, NUM_ELMS:u32 = {NUM_ROWS*NUM_COLS}>(
   in_img: F32[NUM_ELMS],
   row_idx:u32, col_idx:u32, stencil: F32[3][3]) -> F32 {
 

xls/modules/zstd/block_dec.x

@@ -48,7 +48,7 @@ type SequenceExecutorMessageType = common::SequenceExecutorMessageType;
 //   │         └───────────────────┘         │
 //   └───────────────────────────────────────┘
 
-proc BlockDecoder {
+pub proc BlockDecoder {
     input_r: chan<BlockDataPacket> in;
     output_s: chan<SequenceExecutorPacket> out;
 

xls/modules/zstd/ram_printer.x

@@ -22,7 +22,7 @@ enum RamPrinterStatus : u2 {
 
 struct RamPrinterState<ADDR_WIDTH: u32> { status: RamPrinterStatus, addr: bits[ADDR_WIDTH] }
 
-proc RamPrinter<DATA_WIDTH: u32, SIZE: u32, NUM_PARTITIONS: u32, ADDR_WIDTH: u32, NUM_MEMORIES: u32>
+pub proc RamPrinter<DATA_WIDTH: u32, SIZE: u32, NUM_PARTITIONS: u32, ADDR_WIDTH: u32, NUM_MEMORIES: u32>
 {
     print_r: chan<()> in;
     finish_s: chan<()> out;

xls/modules/zstd/repacketizer.x

@@ -37,7 +37,7 @@ const ZERO_ZSTD_DECODED_PACKET = zero!<ZstdDecodedPacket>();
 const ZERO_REPACKETIZER_STATE = zero!<RepacketizerState>();
 const INIT_REPACKETIZER_STATE = RepacketizerState {to_fill: DATA_WIDTH, ..ZERO_REPACKETIZER_STATE};
 
-proc Repacketizer {
+pub proc Repacketizer {
     input_r: chan<ZstdDecodedPacket> in;
     output_s: chan<ZstdDecodedPacket> out;
 

xls/modules/zstd/sequence_executor.x

@@ -807,7 +807,7 @@ fn handle_reapeated_offset_for_sequences
     (seq, final_repeat_offsets)
 }
 
-proc SequenceExecutor<HISTORY_BUFFER_SIZE_KB: u32,
+pub proc SequenceExecutor<HISTORY_BUFFER_SIZE_KB: u32,
      RAM_SIZE: u32 = {ram_size(HISTORY_BUFFER_SIZE_KB)},
      RAM_ADDR_WIDTH: u32 = {ram_addr_width(HISTORY_BUFFER_SIZE_KB)},
      INIT_HB_PTR_ADDR: u32 = {u32:0}, INIT_HB_PTR_RAM: u32 = {u32:0},